Redirections let the server write files and run pipelines, and execute
lets it read files.
Before 4.2.0, the client simply trusted the server. 4.2.0 added
fairly complex code to recognize redirections and execute, replace the
filenames and pipelines by tag strings, remember tag string and
replaced text, and honor redirection and execute only when their text
is a known tag string. Tag and replaced text were freed on use.
Broken by design because the client cannot know whether a line will
actually be read as a command by the server. Issues included:
(1) Non-command lines could be messed up.
(2) The memory used for remembering their tags was never freed.
(3) execute prompting for its argument was incorrectly rejected.
(4) A rogue server could use a tag for the wrong purpose. For
instance, "execute fire" creates a tag for "fire", which a rogue
server could use for a pipeline to command "ire".
4.2.10 dropped the tag strings, and used the actual text as key. This
took care of (1).
Commit
17d6997 and commit
2456a71 (both v4.3.11) tightened checking of
redirections, which took care of (4) for redirections, but not
execute. Relatively harmless, because redirection text always starts
with '>' or '|', but filenames rarely do.
Commit
8b7d0b9 (v4.3.11) replaced the protection code wholesale.
Instead of attempting to recognize redirections and execute, we now
save everything in a ring buffer, and require redirections and execute
to match at a line end in the ring buffer. Much simpler, takes care
of issues (2) and (3), but adds new issues:
(5) When sent-ahead input exceeds the ring buffer, good redirections
and executes get rejected. Could be avoided by limiting send-ahead,
or remembering input until its output arrives. However, bogus
rejections haven't been a problem in practice even with a tiny 4KiB
ring buffer.
(6) The protection against rogue execute is *much* weaker, because we
now accept any line suffix. Before, we accepted any tag,
i.e. anything that looks like a redirection or an execute command.
(7) When we find a match in the ring buffer, we used to drop
everything up to that line right away. This broke redirected execute
commands. Commit
02a9af0 (v4.3.11) fixed it by delaying the drop
until the next prompt, but that's overly complicated.
This commit addresses (7): don't drop on use, simply let new input
push old input out of the ring buffer.
The next commit will address (6) and the remainder of (4).
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
* secure.c: Check redir etc. to protect against tampering deity
*
* Known contributors to this file:
- * Markus Armbruster, 2007
+ * Markus Armbruster, 2007-2015
*/
#include <config.h>
#include "secure.h"
static struct ring recent_input;
-static size_t saved_bytes;
/*
* Remember line of input @inp for a while.
* It must end with a newline.
- * Return value is suitable for forget_input(): it makes it forget all
- * input up to and including this line.
*/
-size_t
+void
save_input(char *inp)
{
size_t len = strlen(inp);
assert(eol >= 0);
ring_discard(&recent_input, eol + 1);
}
- saved_bytes += len;
- return saved_bytes;
}
/*
* Can you still remember a line of input that ends with @tail?
* It must end with a newline.
- * Return non-zero iff @tail can be remembered.
- * Passing that value to forget_input() will forget all input up to
- * and including this line.
*/
-size_t
+int
seen_input(char *tail)
{
size_t len = strlen(tail);
- size_t remembered = ring_len(&recent_input);
- int dist;
assert(len && tail[len - 1] == '\n');
-
- dist = ring_search(&recent_input, tail);
- if (dist < 0)
- return 0;
-
- assert(dist + len <= remembered && remembered <= saved_bytes);
- return saved_bytes - remembered + dist + len;
-}
-
-/*
- * Forget remembered input up to @seen.
- * @seen should be obtained from save_input() or seen_input().
- */
-void
-forget_input(size_t seen)
-{
- size_t forgotten = saved_bytes - ring_len(&recent_input);
-
- assert(seen);
-
- if (seen > forgotten) {
- assert(ring_peek(&recent_input, seen - forgotten - 1) == '\n');
- ring_discard(&recent_input, seen - forgotten);
- }
+ return ring_search(&recent_input, tail) >= 0;
}
#include <stddef.h>
-extern size_t save_input(char *);
-extern size_t seen_input(char *);
-extern void forget_input(size_t);
+extern void save_input(char *);
+extern int seen_input(char *);
#endif
static FILE *redir_fp;
static int redir_is_pipe;
static int executing;
-static size_t input_to_forget;
static void prompt(int, char *, char *);
static void doredir(char *p);
(void)fclose(redir_fp);
redir_fp = NULL;
}
- if (input_to_forget) {
- forget_input(input_to_forget);
- input_to_forget = 0;
- }
prompt(code, the_prompt, teles);
executing = 0;
break;
static int
redir_authorized(char *arg, char *attempt, int expected)
{
- size_t seen = seen_input(arg);
-
if (!expected) {
fprintf(stderr, "Warning: dropped conflicting %s %s",
attempt, arg);
return 0;
}
- if (!seen || (input_to_forget && input_to_forget != seen)) {
+ if (!seen_input(arg)) {
fprintf(stderr, "Warning: server attempted to %s %s",
attempt, arg);
return 0;
}
- input_to_forget = seen;
if (restricted) {
fprintf(stderr, "Can't %s in restricted mode\n", attempt);
projects / empserver / commitdiff