From 394eebed6656dfc2e56a79500b602023000ac415 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Mon, 25 Feb 2008 20:30:24 +0000 Subject: [PATCH] lpd: spool mode added by Vladimir lpr: more robust error reporting *: introduce and use xchroot libbb: full_read/write now will report partial data counts prior to error isdirectory.c: style fixes lpd_main 249 486 +237 xchroot - 29 +29 get_response_or_say_and_die 110 139 +29 full_write 52 60 +8 full_read 55 63 +8 static.newline 1 - -1 switch_root_main 404 400 -4 chpst_main 1089 1079 -10 getopt32 1370 1359 -11 chroot_main 115 101 -14 ------------------------------------------------------------------------------ (add/remove: 1/1 grow/shrink: 4/4 up/down: 311/-40) Total: 271 bytes text data bss dec hex filename 798472 728 7484 806684 c4f1c busybox_old 798775 728 7484 806987 c504b busybox_unstripped --- coreutils/chroot.c | 4 +- include/libbb.h | 5 ++ include/usage.h | 2 +- libbb/full_write.c | 8 +- libbb/isdirectory.c | 11 +-- libbb/read.c | 10 ++- libbb/xfuncs.c | 6 ++ printutils/lpd.c | 156 ++++++++++++++++++++++++--------------- printutils/lpr.c | 25 ++++--- runit/chpst.c | 3 +- util-linux/switch_root.c | 3 +- 11 files changed, 147 insertions(+), 86 deletions(-) diff --git a/coreutils/chroot.c b/coreutils/chroot.c index a3e70e925..1198a415a 100644 --- a/coreutils/chroot.c +++ b/coreutils/chroot.c @@ -19,9 +19,7 @@ int chroot_main(int argc, char **argv) } ++argv; - if (chroot(*argv)) { - bb_perror_msg_and_die("cannot change root directory to %s", *argv); - } + xchroot(*argv); xchdir("/"); ++argv; diff --git a/include/libbb.h b/include/libbb.h index 48937c4b1..1ea2e35be 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -315,6 +315,7 @@ void kill_myself_with_sig(int sig) ATTRIBUTE_NORETURN; void xsetgid(gid_t gid); void xsetuid(uid_t uid); void xchdir(const char *path); +void xchroot(const char *path); void xsetenv(const char *key, const char *value); void xunlink(const char *pathname); void xstat(const char *pathname, struct stat *buf); @@ -500,6 +501,8 @@ extern void *xrealloc(void *old, size_t size); extern ssize_t safe_read(int fd, void *buf, size_t count); extern ssize_t nonblock_safe_read(int fd, void *buf, size_t count); +// NB: will return short read on error, not -1, +// if some data was read before error occurred extern ssize_t full_read(int fd, void *buf, size_t count); extern void xread(int fd, void *buf, size_t count); extern unsigned char xread_char(int fd); @@ -514,6 +517,8 @@ 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); extern ssize_t safe_write(int fd, const void *buf, size_t count); +// NB: will return short write on error, not -1, +// if some data was written before error occurred extern ssize_t full_write(int fd, const void *buf, size_t count); extern void xwrite(int fd, const void *buf, size_t count); diff --git a/include/usage.h b/include/usage.h index 7c9a90e77..359f88d27 100644 --- a/include/usage.h +++ b/include/usage.h @@ -2055,7 +2055,7 @@ USE_FEATURE_BRCTL_FANCY("\n" \ "SPOOLDIR" #define lpd_full_usage \ "Example:" \ - "\n tcpsvd -E localhost 515 lpd /var/spool" + "\n tcpsvd -E 0 515 softlimit -m 99999 lpd /var/spool" #define lpq_trivial_usage \ "[-P queue[@host[:port]]] [-U USERNAME] [-d JOBID...] [-fs]" diff --git a/libbb/full_write.c b/libbb/full_write.c index 7bbacb8ac..7503c8b42 100644 --- a/libbb/full_write.c +++ b/libbb/full_write.c @@ -24,8 +24,14 @@ ssize_t full_write(int fd, const void *buf, size_t len) while (len) { cc = safe_write(fd, buf, len); - if (cc < 0) + if (cc < 0) { + if (total) { + /* we already wrote some! */ + /* user can do another write to know the error code */ + return total; + } return cc; /* write() returns -1 on failure. */ + } total += cc; buf = ((const char *)buf) + cc; diff --git a/libbb/isdirectory.c b/libbb/isdirectory.c index b35919869..1d2477f47 100644 --- a/libbb/isdirectory.c +++ b/libbb/isdirectory.c @@ -12,7 +12,7 @@ #include "libbb.h" /* - * Return TRUE if a fileName is a directory. + * Return TRUE if fileName is a directory. * Nonexistent files return FALSE. */ int is_directory(const char *fileName, const int followLinks, struct stat *statBuf) @@ -21,8 +21,8 @@ int is_directory(const char *fileName, const int followLinks, struct stat *statB struct stat astatBuf; if (statBuf == NULL) { - /* set from auto stack buffer */ - statBuf = &astatBuf; + /* use auto stack buffer */ + statBuf = &astatBuf; } if (followLinks) @@ -30,10 +30,7 @@ int is_directory(const char *fileName, const int followLinks, struct stat *statB else status = lstat(fileName, statBuf); - if (status < 0 || !(S_ISDIR(statBuf->st_mode))) { - status = FALSE; - } - else status = TRUE; + status = (status == 0 && S_ISDIR(statBuf->st_mode)); return status; } diff --git a/libbb/read.c b/libbb/read.c index 4ad41d589..575446536 100644 --- a/libbb/read.c +++ b/libbb/read.c @@ -88,8 +88,14 @@ ssize_t full_read(int fd, void *buf, size_t len) while (len) { cc = safe_read(fd, buf, len); - if (cc < 0) - return cc; /* read() returns -1 on failure. */ + if (cc < 0) { + if (total) { + /* we already have some! */ + /* user can do another read to know the error code */ + return total; + } + return cc; /* read() returns -1 on failure. */ + } if (cc == 0) break; buf = ((char *)buf) + cc; diff --git a/libbb/xfuncs.c b/libbb/xfuncs.c index 17760a3c3..18e696a7a 100644 --- a/libbb/xfuncs.c +++ b/libbb/xfuncs.c @@ -577,6 +577,12 @@ void xchdir(const char *path) bb_perror_msg_and_die("chdir(%s)", path); } +void xchroot(const char *path) +{ + if (chroot(path)) + bb_perror_msg_and_die("can't change root directory to %s", path); +} + // Print a warning message if opendir() fails, but don't die. DIR *warn_opendir(const char *path) { diff --git a/printutils/lpd.c b/printutils/lpd.c index ed3d9d100..916fd6213 100644 --- a/printutils/lpd.c +++ b/printutils/lpd.c @@ -1,6 +1,6 @@ /* vi: set sw=4 ts=4: */ /* - * micro lpd - a small non-queueing lpd + * micro lpd * * Copyright (C) 2008 by Vladimir Dronnikov * @@ -8,72 +8,108 @@ */ #include "libbb.h" +// TODO: xmalloc_reads is vulnerable to remote OOM attack! + int lpd_main(int argc, char *argv[]) MAIN_EXTERNALLY_VISIBLE; int lpd_main(int argc, char *argv[]) { - char *s; - - // goto spool directory - // spool directory contains either links to real printer devices or just simple files - // these links or files are called "queues" - if (!argv[1]) - bb_show_usage(); - - xchdir(argv[1]); - - xdup2(1, 2); + int spooling; + char *s, *queue; // read command s = xmalloc_reads(STDIN_FILENO, NULL); - // N.B. we keep things simple - // only "receive job" command is meaningful here... - if (2 == *s) { - char *queue; - - // parse command: "\x2QUEUE_NAME\n" - queue = s + 1; - *strchrnul(s, '\n') = '\0'; - - while (1) { - // signal OK - write(STDOUT_FILENO, "", 1); - // get subcommand - s = xmalloc_reads(STDIN_FILENO, NULL); - if (!s) - return EXIT_SUCCESS; // EOF (probably) - // valid s must be of form: SUBCMD | LEN | SP | FNAME - // N.B. we bail out on any error - // control or data file follows - if (2 == s[0] || 3 == s[0]) { - int fd; - size_t len; - // 2: control file (ignoring), 3: data file - fd = -1; - if (3 == s[0]) - fd = xopen(queue, O_RDWR | O_APPEND); - // get data length - *strchrnul(s, ' ') = '\0'; - len = xatou(s + 1); - // dump exactly len bytes to file, or die - bb_copyfd_exact_size(STDIN_FILENO, fd, len); - close(fd); // NB: can do close(-1). Who cares? - free(s); - // got no ACK? -> bail out - if (safe_read(STDIN_FILENO, s, 1) <= 0 || s[0]) { - // don't talk to peer - it obviously - // don't follow the protocol - return EXIT_FAILURE; - } - } else { - // any other subcommand aborts receiving job - // N.B. abort subcommand itself doesn't contain - // fname so it failed earlier... - break; - } - } + // we understand only "receive job" command + if (2 != *s) { + unsupported_cmd: + printf("Command %02x %s\n", + (unsigned char)s[0], "is not supported"); + return EXIT_FAILURE; } - printf("Command %02x not supported\n", (unsigned char)*s); - return EXIT_FAILURE; + // spool directory contains either links to real printer devices or just simple files + // these links or files are called "queues" + // OR + // if a directory named as given queue exists within spool directory + // then LPD enters spooling mode and just dumps both control and data files to it + + // goto spool directory + if (argv[1]) + xchdir(argv[1]); + + // parse command: "\x2QUEUE_NAME\n" + queue = s + 1; + *strchrnul(s, '\n') = '\0'; + + // protect against "/../" attacks + if (queue[0] == '.' || strstr(queue, "/.")) + return EXIT_FAILURE; + + // queue is a directory -> chdir to it and enter spooling mode + spooling = chdir(queue) + 1; /* 0: cannot chdir, 1: done */ + + xdup2(STDOUT_FILENO, STDERR_FILENO); + + while (1) { + char *fname; + int fd; + // int is easier than ssize_t: can use xatoi_u, + // and can correctly display error returns (-1) + int expected_len, real_len; + + // signal OK + write(STDOUT_FILENO, "", 1); + + // get subcommand + s = xmalloc_reads(STDIN_FILENO, NULL); + if (!s) + return EXIT_SUCCESS; // probably EOF + // we understand only "control file" or "data file" cmds + if (2 != s[0] && 3 != s[0]) + goto unsupported_cmd; + + *strchrnul(s, '\n') = '\0'; + // valid s must be of form: SUBCMD | LEN | SP | FNAME + // N.B. we bail out on any error + fname = strchr(s, ' '); + if (!fname) { + printf("Command %02x %s\n", + (unsigned char)s[0], "lacks filename"); + return EXIT_FAILURE; + } + *fname++ = '\0'; + if (spooling) { + // spooling mode: dump both files + // make "/../" attacks in file names ineffective + xchroot("."); + // job in flight has mode 0200 "only writable" + fd = xopen3(fname, O_CREAT | O_WRONLY | O_TRUNC | O_EXCL, 0200); + } else { + // non-spooling mode: + // 2: control file (ignoring), 3: data file + fd = -1; + if (3 == s[0]) + fd = xopen(queue, O_RDWR | O_APPEND); + } + expected_len = xatoi_u(s + 1); + real_len = bb_copyfd_size(STDIN_FILENO, fd, expected_len); + if (spooling && real_len != expected_len) { + unlink(fname); // don't keep corrupted files + printf("Expected %d but got %d bytes\n", + expected_len, real_len); + return EXIT_FAILURE; + } + // get ACK and see whether it is NUL (ok) + if (read(STDIN_FILENO, s, 1) != 1 || s[0] != 0) { + // don't send error msg to peer - it obviously + // don't follow the protocol, so probably + // it can't understand us either + return EXIT_FAILURE; + } + // chmod completely downloaded job as "readable+writable" + if (spooling) + fchmod(fd, 0600); + close(fd); // NB: can do close(-1). Who cares? + free(s); + } /* while (1) */ } diff --git a/printutils/lpr.c b/printutils/lpr.c index 52983bfb7..09fbc6ad1 100644 --- a/printutils/lpr.c +++ b/printutils/lpr.c @@ -19,19 +19,26 @@ */ static void get_response_or_say_and_die(const char *errmsg) { - static const char newline = '\n'; - char buf = ' '; + ssize_t sz; + char buf[128]; fflush(stdout); - safe_read(STDOUT_FILENO, &buf, 1); - if ('\0' != buf) { + buf[0] = ' '; + sz = safe_read(STDOUT_FILENO, buf, 1); + if ('\0' != buf[0]) { // request has failed - bb_error_msg("error while %s. Server said:", errmsg); - safe_write(STDERR_FILENO, &buf, 1); - logmode = 0; /* no error messages from bb_copyfd_eof() pls */ - bb_copyfd_eof(STDOUT_FILENO, STDERR_FILENO); - safe_write(STDERR_FILENO, &newline, 1); + // try to make sure last char is '\n', but do not add + // superfluous one + sz = full_read(STDOUT_FILENO, buf + 1, 126); + bb_error_msg("error while %s%s", errmsg, + (sz > 0 ? ". Server said:" : "")); + if (sz > 0) { + // sz = (bytes in buf) - 1 + if (buf[sz] != '\n') + buf[++sz] = '\n'; + safe_write(STDERR_FILENO, buf, sz + 1); + } xfunc_die(); } } diff --git a/runit/chpst.c b/runit/chpst.c index 0f605cc45..7b70a4d5a 100644 --- a/runit/chpst.c +++ b/runit/chpst.c @@ -332,8 +332,7 @@ int chpst_main(int argc, char **argv) if (env_dir) edir(env_dir); if (root) { xchdir(root); - if (chroot(".") == -1) - bb_perror_msg_and_die("chroot"); + xchroot("."); } slimit(); if (nicelvl) { diff --git a/util-linux/switch_root.c b/util-linux/switch_root.c index bd1e9d5ce..3a1741179 100644 --- a/util-linux/switch_root.c +++ b/util-linux/switch_root.c @@ -105,8 +105,9 @@ int switch_root_main(int argc, char **argv) // Overmount / with newdir and chroot into it. The chdir is needed to // recalculate "." and ".." links. - if (mount(".", "/", NULL, MS_MOVE, NULL) || chroot(".")) + if (mount(".", "/", NULL, MS_MOVE, NULL)) bb_error_msg_and_die("error moving root"); + xchroot("."); xchdir("/"); // If a new console specified, redirect stdin/stdout/stderr to that.