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

@@ -19,9 +19,7 @@ int chroot_main(int argc, char **argv)
 	}
 
 	++argv;
-	if (chroot(*argv)) {
+	xchroot(*argv);
-		bb_perror_msg_and_die("cannot change root directory to %s", *argv);
-	}
 	xchdir("/");
 
 	++argv;

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);
 

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]"

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;

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,7 +21,7 @@ int is_directory(const char *fileName, const int followLinks, struct stat *statB
 	struct stat astatBuf;
 
 	if (statBuf == NULL) {
-	    /* set from auto stack buffer */
+		/* use auto stack buffer */
 		statBuf = &astatBuf;
 	}
 
@@ -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 = (status == 0 && S_ISDIR(statBuf->st_mode));
-	    status = FALSE;
-	}
-	else status = TRUE;
 
 	return status;
 }

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)
+		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;

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)
 {

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 <dronnikov@gmail.com>
  *
@@ -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;
+	int spooling;
-
+	char *s, *queue;
-	// 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);
 
 	// read command
 	s = xmalloc_reads(STDIN_FILENO, NULL);
 
-	// N.B. we keep things simple
+	// we understand only "receive job" command
-	// only "receive job" command is meaningful here...
+	if (2 != *s) {
-	if (2 == *s) {
+ unsupported_cmd:
-		char *queue;
+		printf("Command %02x %s\n",
+			(unsigned char)s[0], "is not supported");
+		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; // EOF (probably)
+			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
-			// control or data file follows
+		fname = strchr(s, ' ');
-			if (2 == s[0] || 3 == s[0]) {
+		if (!fname) {
-				int fd;
+			printf("Command %02x %s\n",
-				size_t len;
+				(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);
-				// get data length
+		}
-				*strchrnul(s, ' ') = '\0';
+		expected_len = xatoi_u(s + 1);
-				len = xatou(s + 1);
+		real_len = bb_copyfd_size(STDIN_FILENO, fd, expected_len);
-				// dump exactly len bytes to file, or die
+		if (spooling && real_len != expected_len) {
-				bb_copyfd_exact_size(STDIN_FILENO, fd, 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);
-				// got no ACK? -> bail out
+	} /* while (1) */
-				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;
-			}
-		}
-	}
-
-	printf("Command %02x not supported\n", (unsigned char)*s);
-	return EXIT_FAILURE;
 }

printutils/lpr.c

@@ -19,19 +19,26 @@
  */
 static void get_response_or_say_and_die(const char *errmsg)
 {
-	static const char newline = '\n';
+	ssize_t sz;
-	char buf = ' ';
+	char buf[128];
 
 	fflush(stdout);
 
-	safe_read(STDOUT_FILENO, &buf, 1);
+	buf[0] = ' ';
-	if ('\0' != buf) {
+	sz = safe_read(STDOUT_FILENO, buf, 1);
+	if ('\0' != buf[0]) {
 		// request has failed
-		bb_error_msg("error while %s. Server said:", errmsg);
+		// try to make sure last char is '\n', but do not add
-		safe_write(STDERR_FILENO, &buf, 1);
+		// superfluous one
-		logmode = 0; /* no error messages from bb_copyfd_eof() pls */
+		sz = full_read(STDOUT_FILENO, buf + 1, 126);
-		bb_copyfd_eof(STDOUT_FILENO, STDERR_FILENO);
+		bb_error_msg("error while %s%s", errmsg,
-		safe_write(STDERR_FILENO, &newline, 1);
+				(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();
 	}
 }

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)
+		xchroot(".");
-			bb_perror_msg_and_die("chroot");
 	}
 	slimit();
 	if (nicelvl) {

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.