Re: cygstart patch

[Charles Wilson <cygwin at cwilson dot fastmail dot fm>]

Derosa, Anthony CIV NAVAIR 2035, 2, 205/214 wrote:
> I found a small bug and added a feature to the cygstart utility, which is part of the cygutils package.  The feature that I added removes the limit on the length of the command line arguments passed to the target application, which was previously limited to MAX_PATH.  The bug I fixed was in regard to freeing the variable "args" instead of tyring to free "workDir" twice.  A patch and change log follow below.  As this is my first contribution, please correct me if I did something incorrectly.

Thanks for the patch, Anthony.  It looks good (I can't beLIEVE the 
thinko on free(workDir) )  -- but I have one concern.  Maybe it's 
overblown, but replacing all of the strNcat calls (which are somewhat 
resistant to buffer overruns) with strcat (which are NOT) seems to be 
mistake.

OTOH, I can't see anyway around it without ALSO calling strlen() -- and 
if strlen() succeeds without "going off into the weeds", the strcat() 
will too, so you might as well just use strcat since strNcat+strlen is 
not really any more safe.

I'm not an expert on these issues; anybody else want to chime in here 
before I apply Anthony's patch?

--
Chuck



> --- ../cygutils-1.2.6/src/cygstart/cygstart.c	2002-03-16 00:49:44.000000000 -0500
> +++ src/cygstart/cygstart.c	2005-03-02 09:16:00.383625000 -0500
> @@ -340,14 +340,18 @@ int main(int argc, const char **argv)
>  
>      /* Retrieve any arguments */
>      if (rest && *rest) {
> -        if ((args = (char *) malloc(MAX_PATH+1)) == NULL) {
> +        if ((args = (char *) malloc(strlen(*rest)+1)) == NULL) {
>              fprintf(stderr, "%s: memory allocation error\n", argv[0]);
>              exit(1);
> -        }
> -        strncpy(args, *rest, MAX_PATH);
> +        }	 
> +        strcpy(args, *rest);
>          while (rest++ && *rest) {
> -            strncat(args, " ", MAX_PATH-strlen(args));
> -            strncat(args, *rest, MAX_PATH-strlen(args));
> +            if ( (args = (char *) realloc(args, strlen(args) + strlen(*rest) + 1)) == NULL) {
> +                fprintf(stderr, "%s: memory allocation error\n", argv[0]);
> +                exit(1);
> +            }		
> +            strcat(args, " ");
> +            strcat(args, *rest);
>          }
>      }
>  
> @@ -359,7 +363,7 @@ int main(int argc, const char **argv)
>      if (action)
>          free(action);
>      if (args)
> -        free(workDir);
> +        free(args);

D'oh!


>      if (workDir)
>          free(workDir);
>      if (file)


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/