Bug#448139: cfagent crashes with a segmentation fault when 31+ arguments are supplied

[Iain Patterson]

Quoth Mark Burgess,

This is probably true, but the case cited in the mail is not a limitation of cfengine but the argument "getopt" code in the

operating system concerned. It is normal that Unix limits to 31
args, and internally cfengine does the same for consistency.

  I tried compiling the getopt example code at
http://www.gnu.org/software/libc/manual/html_node/Getopt.html and
didn't get a segfault with >31 arguments on Linux x86_64.

  cfagent 2.2.1 on the same OS did indeed segfault with >31 arguments.

The conclusion is that, while I agree there should be no segfault,
I am not sure what I can do about it, since there is no code in
cfengine that lead to this fault.

  It seems the bug is in the code which preparses arguments.  Line 253
of cfagent.c (at least in version 2.2.1) describes the case where a
script is called with the shell magic

    #!/bla/cfengine -v -f

and this ends up being passed in argv[1] as a single string.  cfagent
then parses this into an array of size CF_MAXARGS, which is 31 as you
state.

  The attached quick and very dirty patch does the preparse in two
stages, first allocating a dynamic buffer big enough for the arguments
and then filling it in as before.

  An alternative option (if you excuse the pun) would be to use popt,
which has the function poptParseArgvString().  The downside is that
this is an added dependency.

--- cfengine-2.2.1/src/cf.defs.h.orig	2007-11-30 08:24:22.122979592 +0000
+++ cfengine-2.2.1/src/cf.defs.h	2007-11-30 08:24:26.734643977 +0000
@@ -287,7 +287,6 @@
 #define CF_NONCELEN (CF_BUFSIZE/16)
 #define CF_MAXLINKSIZE 256
 #define CF_MAXLINKLEVEL 4
-#define CF_MAXARGS 31
 #define CF_MAXFARGS 8
 #define CF_MAX_IP_LEN 64       /* numerical ip length */
 #define CF_PROCCOLS 16
--- cfengine-2.2.1/src/cfagent.c.orig	2007-11-30 08:24:04.652251054 +0000
+++ cfengine-2.2.1/src/cfagent.c	2007-11-30 08:47:17.382186413 +0000
@@ -198,8 +198,8 @@
  
 void Initialize(int argc,char *argv[])
 
-{ char *sp, *cfargv[CF_MAXARGS];
-  int i, cfargc, seed;
+{ char *sp, **cfargv;
+  int i, j, cfargc, seed;
   struct stat statbuf;
   unsigned char s[16];
   char ebuff[CF_EXPANDSIZE];
@@ -235,9 +235,40 @@
 /* Everything ends up inside a single argument! Here's the fix      */
 
 cfargc = 1;
-cfargv[0]="cfagent";
 
-for (i = 1; i < argc; i++)
+/* Pass 1: Find how many arguments there are. */
+for (i = 1, j = 1; i < argc; i++)
+   {
+   sp = argv[i];
+   
+   while (*sp != '\0')
+      {
+      while (*sp == ' ' && *sp != '\0') /* Skip to arg */
+         {
+         sp++;
+         }
+      
+      cfargc++;
+      
+      while (*sp != ' ' && *sp != '\0') /* Skip to white space */
+         {
+         sp++;
+         }
+      }
+   }
+
+/* Allocate memory for cfargv. */
+cfargv = (char **) malloc(sizeof(char *) * cfargc + 1);
+if (! cfargv)
+   {
+   /* I guess CfLog(cferror, ...) isn't available yet. */
+   fprintf(stderr,"cfagent: Out of memory parsing arguments\n");
+   exit(111);
+   }
+
+/* Pass 2: Parse the arguments. */
+cfargv[0] = "cfagent";
+for (i = 1, j = 1; i < argc; i++)
    {
    sp = argv[i];
    
@@ -252,7 +283,7 @@
          sp++;
          }
       
-      cfargv[cfargc++] = sp;
+      cfargv[j++] = sp;
       
       while (*sp != ' ' && *sp != '\0') /* Skip to white space */
          {
@@ -260,6 +291,7 @@
          }
       }
    }
+cfargv[j] = 0;
  
  VDEFAULTBINSERVER.name = "";
  
@@ -315,6 +347,7 @@
  seed = ElfHash(s);
  srand48((long)seed);  
  CheckOpts(cfargc,cfargv);
+ free(cfargv);
 
  AddInstallable("no_default_route");
  CfenginePort();