This is a variety of fixes to BuildCommandLine to avoid buffer overflows and
the like. Before, the copy functions didn't check whether they were
overflowwing CF_BUFFSIZE*2 (the size given to the install string by
InstallPackage), and it also would pass off commands to cfpopen that had more
than CF_MAXSHELLARGS arguments, meaning that the commands would consistantly
fail if you had more than about 15 packages to install with a few package
manager flags thrown in.
I've also removed the code that attempts to run the package command without the
tail string (any characters after %s), as this won't work in all situations,
for examlpe: MyPackageInstallCommand = ( "/bin/sh -c 'realinstallcmd %s |
filter'" ). Sure, running it through sh might be a security weakness on the
person with that setup, but having the code try to execl "/bin/sh -c
'realinstallcmd" can have strange side effects. There's sufficent warning if
the command fails in InstallPackage, I don't think any is needed here.
Index: cfengine-trunk/src/package.c
===================================================================
--- cfengine-trunk.orig/src/package.c
+++ cfengine-trunk/src/package.c
@@ -403,13 +403,13 @@ int BuildCommandLine(char *resolvedcmd,
{ struct Item *package;
char *cmd_tail;
FILE *pp;
- char *ptr, *cmd_ptr;
+ char *cmd_ptr;
int cmd_args = 0;
int cmd_tail_len = 0;
- int original_len;
+ int cmd_head_len = 0;
/* How many words are there in the package manager invocation? */
- for (ptr = rawcmd, cmd_args = 1; NULL != ptr; ptr = strchr(++ptr, ' '))
+ for (cmd_ptr = rawcmd, cmd_args = 1; NULL != cmd_ptr; cmd_ptr =
strchr(++cmd_ptr, ' '))
{
++cmd_args;
}
@@ -428,39 +428,39 @@ int BuildCommandLine(char *resolvedcmd,
/* Copy the initial part of the install command */
strncpy(resolvedcmd, rawcmd, CF_BUFSIZE*2);
- snprintf(OUTPUT,CF_BUFSIZE,"Package manager will be invoked as %s\n",
resolvedcmd);
- CfLog(cfinform,OUTPUT,"");
-
- if ((pp = cfpopen(resolvedcmd, "r")) == NULL)
- {
- Verbose("Could not execute package manager\n");
- /* Return that the package is still not installed */
- return 0;
- }
-
/* Iterator through package list until we reach the maximum number that
cfpopen can take */
- original_len = strlen(resolvedcmd);
- cmd_ptr = &resolvedcmd[original_len];
+ cmd_head_len = strlen(resolvedcmd);
+ cmd_ptr = &resolvedcmd[cmd_head_len];
for (package = pkglist; package != NULL; package = package->next)
{
- Verbose("BuildCommandLine(): Adding package '%s' to the list.\n",
package->name);
+ if (cmd_args == CF_MAXSHELLARGS)
+ {
+ Verbose("BuildCommandLine(): Skipping package %s (reached
CF_MAXSHELLARGS)\n", package->name);
+ }
+ else if ((int) cmd_ptr - (int) resolvedcmd + strlen(package->name) + 2 >
CF_BUFSIZE*2)
+ {
+ Verbose("BuildCommandLine(): Skipping package %s (reached 2 *
CF_BUFFSIZE)\n", package->name);
+ }
+ else
+ {
+ Verbose("BuildCommandLine(): Adding package '%s' to the list.\n",
package->name);
- strncpy(cmd_ptr, package->name, &resolvedcmd[CF_BUFSIZE*2] - cmd_ptr);
- cmd_ptr += strlen(package->name);
- *cmd_ptr++ = ' ';
+ strncpy(cmd_ptr, package->name, &resolvedcmd[CF_BUFSIZE*2] - cmd_ptr);
+ cmd_ptr += strlen(package->name);
+ *cmd_ptr++ = ' ';
+ ++cmd_args;
+ }
}
/* Remove trailing space */
*--cmd_ptr = '\0';
- if (cmd_ptr != &resolvedcmd[original_len])
+ /* Append tail if necessary */
+ if (cmd_tail_len > 0)
{
- /* Have a full command line, so append the tail if necessary. */
- if (cmd_tail_len > 0)
- {
- strncpy(cmd_ptr, cmd_tail, &resolvedcmd[CF_BUFSIZE*2] - cmd_ptr);
- }
+ strncpy(cmd_ptr, cmd_tail, &resolvedcmd[CF_BUFSIZE*2] - cmd_ptr);
}
+
return 1;
}
--
Eric Searcy
OSU Open Source Lab
_______________________________________________
Bug-cfengine mailing list
[email protected]
https://cfengine.org/mailman/listinfo/bug-cfengine
