From 0b6c6a9c9f555a33d681290cce77510460457c03 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Mon, 24 Mar 2008 00:04:42 +0000 Subject: lpd: fix OOM vulnerability (was eating arbitrarily large commands) --- printutils/lpd.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) (limited to 'printutils') 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 -- cgit v1.2.3