From 11350529fee2dbbcfda333bbe10d72f023dc2109 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Mon, 20 Apr 2009 15:13:34 +0100 Subject: [PATCH] Various fixes to the daemon: - make sure SIGPIPE doesn't kill us - warn not to use macros in FileIn functions - add shell_quote function --- daemon/daemon.h | 19 +++++++++++++++---- daemon/guestfsd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index bd14b12..3f51056 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -44,6 +44,8 @@ extern int command (char **stdoutput, char **stderror, const char *name, ...); extern int commandv (char **stdoutput, char **stderror, char * const* const argv); +extern int shell_quote (char *out, int len, const char *in); + extern int verbose; /*-- in proto.c --*/ @@ -90,7 +92,9 @@ extern void send_file_end (int cancel); /* only call this if there is a FileOut parameter */ extern void reply (xdrproc_t xdrp, char *ret); -/* Helper for functions that need a root filesystem mounted. */ +/* Helper for functions that need a root filesystem mounted. + * NB. Cannot be used for FileIn functions. + */ #define NEED_ROOT(errcode) \ do { \ if (!root_mounted) { \ @@ -100,7 +104,9 @@ extern void reply (xdrproc_t xdrp, char *ret); } \ while (0) -/* Helper for functions that need an argument ("path") that is absolute. */ +/* Helper for functions that need an argument ("path") that is absolute. + * NB. Cannot be used for FileIn functions. + */ #define ABS_PATH(path,errcode) \ do { \ if ((path)[0] != '/') { \ @@ -109,7 +115,9 @@ extern void reply (xdrproc_t xdrp, char *ret); } \ } while (0) -/* Helper for functions that need an argument ("path") that is a device. */ +/* Helper for functions that need an argument ("path") that is a device. + * NB. Cannot be used for FileIn functions. + */ #define IS_DEVICE(path,errcode) \ do { \ struct stat statbuf; \ @@ -125,6 +133,7 @@ extern void reply (xdrproc_t xdrp, char *ret); /* Helper for functions which need either an absolute path in the * mounted filesystem, OR a /dev/ device which exists. + * NB. Cannot be used for FileIn functions. */ #define NEED_ROOT_OR_IS_DEVICE(path,errcode) \ do { \ @@ -147,7 +156,9 @@ extern void reply (xdrproc_t xdrp, char *ret); #define CHROOT_OUT \ do { int old_errno = errno; chroot ("."); errno = old_errno; } while (0) -/* Marks functions which are not implemented. */ +/* Marks functions which are not implemented. + * NB. Cannot be used for FileIn functions. + */ #define XXX_NOT_IMPL(errcode) \ do { \ reply_with_error ("%s: function not implemented", __func__); \ diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index a957683..d7ba482 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -32,6 +32,8 @@ #include #include #include +#include +#include #include "daemon.h" @@ -66,6 +68,7 @@ main (int argc, char *argv[]) struct addrinfo hints; XDR xdr; uint32_t len; + struct sigaction sa; for (;;) { c = getopt_long (argc, argv, options, long_options, NULL); @@ -140,6 +143,13 @@ main (int argc, char *argv[]) port = VMCHANNEL_PORT; } + /* Make sure SIGPIPE doesn't kill us. */ + memset (&sa, 0, sizeof sa); + sa.sa_handler = SIG_IGN; + sa.sa_flags = 0; + if (sigaction (SIGPIPE, &sa, NULL) == -1) + perror ("sigaction SIGPIPE"); /* but try to continue anyway ... */ + /* Resolve the hostname. */ memset (&hints, 0, sizeof hints); hints.ai_socktype = SOCK_STREAM; @@ -510,3 +520,40 @@ commandv (char **stdoutput, char **stderror, char * const* const argv) } else return -1; } + +/* Quote 'in' for the shell, and write max len-1 bytes to out. The + * result will be NUL-terminated, even if it is truncated. + * + * Returns number of bytes needed, so if result >= len then the buffer + * should have been longer. + * + * XXX This doesn't quote \n correctly (but is still safe). + */ +int +shell_quote (char *out, int len, const char *in) +{ +#define SAFE(c) (isalnum((c)) || \ + (c) == '/' || (c) == '-' || (c) == '_' || (c) == '.') + int i, j; + int outlen = strlen (in); + + /* Calculate how much output space this really needs. */ + for (i = 0; in[i]; ++i) + if (!SAFE (in[i])) outlen++; + + /* Now copy the string, but only up to len-1 bytes. */ + for (i = 0, j = 0; in[i]; ++i) { + int is_safe = SAFE (in[i]); + + /* Enough space left to write this character? */ + if (j >= len-1 || (!is_safe && j >= len-2)) + break; + + if (!is_safe) out[j++] = '\\'; + out[j++] = in[i]; + } + + out[j] = '\0'; + + return outlen; +} -- 1.8.3.1