client: Fix rogue execute protection

To protect against a rogue server reading your files, the client
honors C_EXECUTE only when it matches recent player input.

This has a somewhat troubled history, detailed in the previous commit.

The remaining major issue comes from commit 8b7d0b9 (v4.3.11): any
suffix of a recent line of input is accepted as C_EXECUTE text.
Before, only text that looked like an argument of an execute command
or a redirection was accepted.

Fix by again requiring the text to be preceded by something that looks
like an execute command.  But do it more carefully: don't break
execute with a prompted for argument, and prevent abuse of
redirections for execute.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>

src/client/ringbuf.c

@@ -157,16 +157,17 @@ ring_discard(struct ring *r, int n)
 
 /*
  * Search the ring buffer for zero-terminated string S.
- * If found, return a non-negative value referring to the beginning of
- * S in the buffer when passed to ring_peek().  Else return -1.
+ * Start at the @(n+1)-th byte to be gotten.
+ * If found, return the number of bytes in the buffer before S.
+ * Else return -1.
  */
 int
-ring_search(struct ring *r, char *s)
+ring_search(struct ring *r, char *s, int n)
 {
     size_t len = strlen(s);
     size_t i, j;
 
-    for (i = r->cons; i + len <= r->prod; i++) {
+    for (i = r->cons + n; i + len <= r->prod; i++) {
 	for (j = 0; s[j] && s[j] == (char)r->buf[(i + j) % RING_SIZE]; j++)
 	    ;
 	if (!s[j])

src/client/ringbuf.h

@@ -27,7 +27,7 @@
  *  ringbuf.h: Simple ring buffer
  *
  *  Known contributors to this file:
- *     Markus Armbruster, 2007
+ *     Markus Armbruster, 2007-2015
  */
 
 #ifndef RINGBUF_H
@@ -59,7 +59,7 @@ extern int ring_getc(struct ring *);
 extern int ring_putc(struct ring *, unsigned char);
 extern int ring_putm(struct ring *, void *, size_t);
 extern void ring_discard(struct ring *, int);
-extern int ring_search(struct ring *, char *);
+extern int ring_search(struct ring *, char *, int);
 extern int ring_from_file(struct ring *, int fd);
 extern int ring_to_file(struct ring *, int fd);
 

src/client/secure.c

@@ -33,6 +33,8 @@
 #include <config.h>
 
 #include <assert.h>
+#include <ctype.h>
+#include <stdio.h>
 #include <string.h>
 #include "ringbuf.h"
 #include "secure.h"
@@ -52,7 +54,7 @@ save_input(char *inp)
     assert(len && inp[len - 1] == '\n');
 
     while (ring_putm(&recent_input, inp, len) < 0) {
-	eol = ring_search(&recent_input, "\n");
+	eol = ring_search(&recent_input, "\n", 0);
 	assert(eol >= 0);
 	ring_discard(&recent_input, eol + 1);
     }
@@ -68,5 +70,42 @@ seen_input(char *tail)
     size_t len = strlen(tail);
 
     assert(len && tail[len - 1] == '\n');
-    return ring_search(&recent_input, tail) >= 0;
+    return ring_search(&recent_input, tail, 0) >= 0;
+}
+
+/*
+ * Can you still remember input that looks like an execute @arg?
+ * @arg must end with a newline.
+ */
+int
+seen_exec_input(char *arg)
+{
+    size_t len = strlen(arg);
+    int n, i, j, ch;
+    unsigned char buf[RING_SIZE + 1];
+
+    assert(len && arg[len - 1] == '\n');
+
+    n = 1;
+    for (;;) {
+	/* find next line ending with arg */
+	n = ring_search(&recent_input, arg, n + 1);
+	if (n <= 0)
+	    return 0;
+
+	/* extract command (same or previous line) */
+	i = n - 1;
+	if (ring_peek(&recent_input, i) == '\n')
+	    i--;
+	j = sizeof(buf);
+	buf[--j] = 0;
+	for (; i >= 0 && (ch = ring_peek(&recent_input, i)) != '\n'; i--)
+	    buf[--j] = ch;
+
+	/* compare command */
+	for (; isspace(buf[j]); j++) ;
+	for (i = j; buf[i] && !isspace(buf[i]); i++) ;
+	if (i - j >= 2 && !strncmp("execute", (char *)buf + j, i - j))
+	    return 1;
+    }
 }

src/client/secure.h

@@ -37,5 +37,6 @@
 
 extern void save_input(char *);
 extern int seen_input(char *);
+extern int seen_exec_input(char *);
 
 #endif

src/client/servcmd.c

@@ -154,20 +154,8 @@ fname(char *s)
 }
 
 static int
-redir_authorized(char *arg, char *attempt, int expected)
+common_authorized(char *arg, char *attempt)
 {
-    if (!expected) {
-	fprintf(stderr, "Warning: dropped conflicting %s %s",
-		attempt, arg);
-	return 0;
-    }
-
-    if (!seen_input(arg)) {
-	fprintf(stderr, "Warning: server attempted to %s %s",
-		attempt, arg);
-	return 0;
-    }
-
     if (restricted) {
 	fprintf(stderr, "Can't %s in restricted mode\n", attempt);
 	return 0;
@@ -180,13 +168,43 @@ redir_authorized(char *arg, char *attempt, int expected)
     return 1;
 }
 
+static int
+redir_authorized(char *arg, char *attempt)
+{
+    if (redir_fp) {
+	fprintf(stderr, "Warning: dropped conflicting %s %s",
+		attempt, arg);
+	return 0;
+    }
+
+    if (!seen_input(arg)) {
+	fprintf(stderr, "Warning: server attempted to %s %s",
+		attempt, arg);
+	return 0;
+    }
+
+    return common_authorized(arg, attempt);
+}
+
+static int
+exec_authorized(char *arg)
+{
+    if (!seen_exec_input(arg)) {
+	fprintf(stderr,
+		"Warning: server attempted to execute batch file %s", arg);
+	return 0;
+    }
+
+    return common_authorized(arg, "execute batch file");
+}
+
 static void
 doredir(char *p)
 {
     int mode;
     int fd;
 
-    if (!redir_authorized(p, "redirect to file", !redir_fp))
+    if (!redir_authorized(p, "redirect to file"))
 	return;
     if (*p++ != '>') {
 	fprintf(stderr, "Warning: dropped weird redirection %s", p);
@@ -221,7 +239,7 @@ doredir(char *p)
 static void
 dopipe(char *p)
 {
-    if (!redir_authorized(p, "pipe to shell command", !redir_fp))
+    if (!redir_authorized(p, "pipe to shell command"))
 	return;
     if (*p++ != '|') {
 	fprintf(stderr, "Warning: dropped weird pipe %s", p);
@@ -250,7 +268,7 @@ doexecute(char *p)
 {
     int fd;
 
-    if (!redir_authorized(p, "execute batch file", 1))
+    if (!exec_authorized(p))
 	return -1;
 
     p = fname(p);