@@ -1,6 +1,8 @@

+  * Fix fd modifier, signal, and exit status parsing to be rigourous in
+    their use of strtoul.  (Thanks to report from Peter Benie.)

diff --git a/client.c b/client.c
index c6b259bbd899c73046766ab8a1e33a572efe5ba5..9fb76df18c34be6822de85c953d57bc3d9a0ef7a 100644 (file)
--- a/client.c
+++ b/client.c
@@ -497,43 +497,99 @@ static const struct fdmodifierinfo fdmodifierinfos[]= {
   {  0                                                                            }
 };
 
-static void addfdmodifier(int fd, const char *key) {
+static void addfdmodifier(int fd, const char *key, size_t key_len) {
   const struct fdmodifierinfo *fdmip;
   
   if (!*key) return;
-  for (fdmip= fdmodifierinfos; fdmip->string && strcmp(fdmip->string,key); fdmip++);
-  if (!fdmip->string) usageerror("unknown fdmodifer `%s' for fd %d",key,fd);
+  for (fdmip= fdmodifierinfos;
+       fdmip->string &&
+        !(strlen(fdmip->string) == key_len &&
+          !memcmp(fdmip->string,key,key_len));
+       fdmip++);
+  if (!fdmip->string) usageerror("unknown fdmodifer `%.*s' for fd %d",
+                                (int)key_len,key,fd);
   if (fdmip->conflicts & fdsetup[fd].mods)
-    usageerror("fdmodifier `%s' conflicts with another for fd %d",key,fd);
+    usageerror("fdmodifier `%.*s' conflicts with another for fd %d",
+              (int)key_len,key,fd);
   fdsetup[fd].mods |= fdmip->implies;
   fdsetup[fd].oflags |= fdmip->oflags;
 }
 
-static int fdstdnumber(const char *string) {
-  if (!strcmp(string,"stdin")) return 0;
-  else if (!strcmp(string,"stdout")) return 1;
-  else if (!strcmp(string,"stderr")) return 2;
-  else return -1;
+static void addfdmodifier_fixed(int fd, const char *key) {
+  addfdmodifier(fd, key, strlen(key));
 }
 
+static int fdstdnumber(const char *string, const char **delim_r) {
+#define FN(v,s) do{                            \
+    if (!memcmp(string,s,sizeof(s)-1)) {       \
+      *delim_r= string+sizeof(s)-1;            \
+      return v;                                        \
+    }                                          \
+  }while(0)
+
+  FN(0,"stdin");
+  FN(1,"stdout");
+  FN(2,"stderr");
+
+  return -1;
+}
+
+static int strtofd(const char *string,
+                  const char **mods_r /* 0: no modifiers, must go to end */,
+                  const char *what) {
+  int fd;
+  unsigned long ul;
+  char *delim_v;
+  const char *mods;
+  
+  fd= fdstdnumber(string,&mods);
+  if (fd>=0) {
+    if (*mods && *mods != ',')
+      usageerror("%s, when it is `stdin', `stdout' or `stderr',"
+                " must be delimited with a comma from any following"
+                " modifiers - `%s' is not permitted",
+                what, mods);
+    goto parsed;
+  }
+
+  errno= 0;
+  ul= strtoul(string,&delim_v,10);
+  if (errno || delim_v == string || ul > INT_MAX)
+    usageerror("%s must be must be numeric file descriptor"
+              " or `stdin', `stdout' or `stderr'"
+              " - `%s' is not recognized",
+              what, string);
+  mods= delim_v;
+  fd= ul;
+
+ parsed:
+  if (*mods==',')
+    mods++;
+  
+  if (mods_r)
+    *mods_r= mods;
+  else if (*mods)
+    usageerror("%s must be must be only file descriptor"
+              " - trailing portion or modifiers `%s' not permitted",
+              what, mods);
+
+  if (fd > MAX_ALLOW_FD)
+    usageerror("%s file descriptor specified (%d)"
+              " is larger than maximum allowed (%d)",
+              what, fd, MAX_ALLOW_FD);
+  
+  return fd;
+}
+  
 static void of_file(const struct optioninfo *oip, const char *value, char *key) {
   unsigned long fd, copyfd;
   struct stat stab;
   int oldarraysize, r;
-  char *delim;
-
-  fd= strtoul(key,&delim,10);
-  if (delim == key) {
-    delim= strchr(key,',');
-    if (delim) *delim++= 0;
-    fd= fdstdnumber(key);
-    if (fd<0) usageerror("first part of argument to -f or --file must be numeric "
-                        "file descriptor or `stdin', `stdout' or `stderr' - `%s' "
-                        "is not recognized",key);
-  }
-  if (fd > MAX_ALLOW_FD)
-    usageerror("file descriptor specified (%lu) is larger than maximum allowed (%d)",
-              fd,MAX_ALLOW_FD);
+  size_t mod_len;
+  const char *mods, *delim;
+
+  fd= strtofd(key,&mods,"first part of argument to -f or --file");
+
   if (fd >= fdsetupsize) {
     oldarraysize= fdsetupsize;
     fdsetupsize+=2; fdsetupsize<<=1;
@@ -554,33 +610,24 @@ static void of_file(const struct optioninfo *oip, const char *value, char *key)
   fdsetup[fd].oflags= 0;
   fdsetup[fd].mods= 0;
   fdsetup[fd].copyfd= -1;
-  while (delim && *delim) {
-    key= delim;
-    delim= strchr(key,',');
-    if (delim) *delim++= 0;
-    addfdmodifier(fd,key);
+  while (mods && *mods) {
+    delim= strchr(mods,',');
+    mod_len= delim ? delim-mods : strlen(mods);
+    addfdmodifier(fd,mods,mod_len);
+    mods= delim ? delim+1 : 0;
   }
   if (!(fdsetup[fd].mods & (fdm_read|fdm_write))) {
     if (fd == 0) {
-      addfdmodifier(fd,"read");
+      addfdmodifier_fixed(fd,"read");
     } else if (fdsetup[fd].mods & fdm_fd) {
-      addfdmodifier(fd,"write");
+      addfdmodifier_fixed(fd,"write");
     } else {
-      addfdmodifier(fd,"overwrite");
+      addfdmodifier_fixed(fd,"overwrite");
     }
   }
   if (fdsetup[fd].mods & fdm_fd) {
-    copyfd= fdstdnumber(value);
-    if (copyfd<0) {
-      copyfd= strtoul(value,&delim,0);
-      if (*delim)
-       usageerror("value part of argument to --file with fd modifier must be "
-                  "numeric or fd name - `%s' is not recognised",value);
-      else if (copyfd > MAX_ALLOW_FD)
-       usageerror("file descriptor %lu named as target of file descriptor redirection"
-                  " (for file descriptor %lu) is larger than maximum allowed (%d)",
-                  copyfd,fd,MAX_ALLOW_FD);
-    }
+    copyfd= strtofd(value,0,
+                   "value part of argument to --file with fd modifier");
     r= fstat(copyfd,&stab);
     if (r) {
       if (oip) fsyscallerror("check filedescriptor %lu (named as target of file "
@@ -593,18 +640,9 @@ static void of_file(const struct optioninfo *oip, const char *value, char *key)
 
 static void of_fdwait(const struct optioninfo *oip, const char *value, char *key) {
   const struct fdmodifierinfo *fdmip;
-  unsigned long ul;
   int fd;
-  char *delim;
   
-  fd= fdstdnumber(key);
-  if (fd<0) {
-    ul= strtoul(key,&delim,0);
-    if (*delim) usageerror("first part of argument to --fdwait must be "
-                          "numeric or fd name - `%s' is not recognised",key);
-    if (ul>MAX_ALLOW_FD) usageerror("first part of argument to --fdwait is too large");
-    fd= ul;
-  }
+  fd= strtofd(key,0,"first part of argument to --fdwait");
   if (fd >= fdsetupsize || !fdsetup[fd].filename)
     usageerror("file descriptor %d specified in --fdwait option is not open",fd);
   for (fdmip= fdmodifierinfos; fdmip->string && strcmp(fdmip->string,value); fdmip++);
@@ -637,9 +675,11 @@ static void of_defvar(const struct optioninfo *oip, const char *value, char *key
 static void of_timeout(const struct optioninfo *oip, const char *value, char *key) {
   char *endp;
   unsigned long ul;
-  
-  ul= strtoul(value,&endp,0);
-  if (*endp) usageerror("timeout value `%s' must be a plain decimal string",value);
+
+  errno= 0;
+  ul= strtoul(value,&endp,10);
+  if (errno || *endp)
+    usageerror("timeout value `%s' must be a plain decimal string",value);
   if (ul>INT_MAX) usageerror("timeout value %lu too large",ul);
   timeout= ul;
 }
@@ -647,9 +687,10 @@ static void of_timeout(const struct optioninfo *oip, const char *value, char *ke
 static void of_signals(const struct optioninfo *oip, const char *value, char *key) {
   unsigned long numvalue;
   char *endp;
-  
-  numvalue= strtoul(value,&endp,0);
-  if (*endp) {
+
+  errno= 0;
+  numvalue= strtoul(value,&endp,10);
+  if (errno || *endp) {
     if (!strcmp(value,"number")) signalsexit= se_number;
     else if (!strcmp(value,"number-nocore")) signalsexit= se_numbernocore;
     else if (!strcmp(value,"highbit")) signalsexit= se_highbit;

diff --git a/debian/changelog b/debian/changelog
index db63aa495a21f672ba8f0af4f84f354c13cf25db..e7e6bf55ddb4a7265355a6c115d1bb3dc6ce3fd2 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,6 +1,8 @@
 userv (1.0.1.99.2) unstable; urgency=low
 
   * Actually ship uservd(8) !
+  * Fix fd modifier, signal, and exit status parsing to be rigourous in
+    their use of strtoul.  (Thanks to report from Peter Benie.)
 
  --