diff options
author | Denis Vlasenko <vda.linux@googlemail.com> | 2008-03-24 03:04:42 +0300 |
---|---|---|
committer | Denis Vlasenko <vda.linux@googlemail.com> | 2008-03-24 03:04:42 +0300 |
commit | 0b6c6a9c9f555a33d681290cce77510460457c03 (patch) | |
tree | 0d5f95c0cc0a2f6945aa97fa50266e8b8288da75 | |
parent | a79428998d76c1758ca12546e5db945a0cd64518 (diff) |
lpd: fix OOM vulnerability (was eating arbitrarily large commands)
-rw-r--r-- | include/libbb.h | 2 | ||||
-rw-r--r-- | libbb/read.c | 9 | ||||
-rw-r--r-- | printutils/lpd.c | 39 | ||||
-rw-r--r-- | shell/hush.c | 2 |
4 files changed, 33 insertions, 19 deletions
diff --git a/include/libbb.h b/include/libbb.h index 9f208b390..07f74e476 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -529,7 +529,7 @@ extern char *reads(int fd, char *buf, size_t count); // Read one line a-la fgets. Reads byte-by-byte. // Useful when it is important to not read ahead. // Bytes are appended to pfx (which must be malloced, or NULL). -extern char *xmalloc_reads(int fd, char *pfx); +extern char *xmalloc_reads(int fd, char *pfx, size_t *maxsz_p); extern ssize_t read_close(int fd, void *buf, size_t count); extern ssize_t open_read_close(const char *filename, void *buf, size_t count); extern void *xmalloc_open_read_close(const char *filename, size_t *sizep); diff --git a/libbb/read.c b/libbb/read.c index 575446536..9c025e3a3 100644 --- a/libbb/read.c +++ b/libbb/read.c @@ -152,13 +152,14 @@ char *reads(int fd, char *buffer, size_t size) // Read one line a-la fgets. Reads byte-by-byte. // Useful when it is important to not read ahead. // Bytes are appended to pfx (which must be malloced, or NULL). -char *xmalloc_reads(int fd, char *buf) +char *xmalloc_reads(int fd, char *buf, size_t *maxsz_p) { char *p; - int sz = buf ? strlen(buf) : 0; + size_t sz = buf ? strlen(buf) : 0; + size_t maxsz = maxsz_p ? *maxsz_p : MAXINT(size_t); goto jump_in; - while (1) { + while (sz < maxsz) { if (p - buf == sz) { jump_in: buf = xrealloc(buf, sz + 128); @@ -178,6 +179,8 @@ char *xmalloc_reads(int fd, char *buf) p++; } *p++ = '\0'; + if (maxsz_p) + *maxsz_p = p - buf - 1; return xrealloc(buf, p - buf); } diff --git a/printutils/lpd.c b/printutils/lpd.c index 45ad6d7e5..fe895939a 100644 --- a/printutils/lpd.c +++ b/printutils/lpd.c @@ -58,8 +58,6 @@ */ #include "libbb.h" -// TODO: xmalloc_reads is vulnerable to remote OOM attack! - // strip argument of bad chars static char *sane(char *str) { @@ -75,6 +73,21 @@ static char *sane(char *str) return str; } +/* vfork() disables some optimizations. Moving its use + * to minimal, non-inlined function saves bytes */ +static NOINLINE void vfork_close_stdio_and_exec(char **argv) +{ + if (vfork() == 0) { + // CHILD + // we are the helper. we wanna be silent. + // this call reopens stdio fds to "/dev/null" + // (no daemonization is done) + bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO | DAEMON_ONLY_SANITIZE, NULL); + BB_EXECVP(*argv, argv); + _exit(127); + } +} + static void exec_helper(const char *fname, char **argv) { char *p, *q, *file; @@ -103,26 +116,24 @@ static void exec_helper(const char *fname, char **argv) // next line, plz! q = p; } + free(file); - if (vfork() == 0) { - // CHILD - // we are the helper. we wanna be silent - // this call reopens stdio fds to "/dev/null" - // (no daemonization is done) - bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO | DAEMON_ONLY_SANITIZE, NULL); - BB_EXECVP(*argv, argv); - _exit(127); - } + vfork_close_stdio_and_exec(argv); // PARENT (or vfork error) // clean up... - free(file); while (--env_idx >= 0) { *strchrnul(our_env[env_idx], '=') = '\0'; unsetenv(our_env[env_idx]); } } +static char *xmalloc_read_stdin(void) +{ + size_t max = 4 * 1024; /* more than enough for commands! */ + return xmalloc_reads(STDIN_FILENO, NULL, &max); +} + int lpd_main(int argc, char *argv[]) MAIN_EXTERNALLY_VISIBLE; int lpd_main(int argc ATTRIBUTE_UNUSED, char *argv[]) { @@ -130,7 +141,7 @@ int lpd_main(int argc ATTRIBUTE_UNUSED, char *argv[]) char *s, *queue; // read command - s = xmalloc_reads(STDIN_FILENO, NULL); + s = xmalloc_read_stdin(); // we understand only "receive job" command if (2 != *s) { @@ -168,7 +179,7 @@ int lpd_main(int argc ATTRIBUTE_UNUSED, char *argv[]) write(STDOUT_FILENO, "", 1); // get subcommand - s = xmalloc_reads(STDIN_FILENO, NULL); + s = xmalloc_read_stdin(); if (!s) return EXIT_SUCCESS; // probably EOF // we understand only "control file" or "data file" cmds diff --git a/shell/hush.c b/shell/hush.c index eb0633b18..545367cb6 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1061,7 +1061,7 @@ static int builtin_read(char **argv) char *string; const char *name = argv[1] ? argv[1] : "REPLY"; - string = xmalloc_reads(STDIN_FILENO, xasprintf("%s=", name)); + string = xmalloc_reads(STDIN_FILENO, xasprintf("%s=", name), NULL); return set_local_var(string, 0); } |