Mail Archives: cygwin/2005/03/03/08:49:24
>OTOH, I can't see anyway around it without ALSO calling strlen()
I'm no expert either, but I did use strlen() when allocating the
memory (with malloc() and realloc()). So if we successfully allocated
enough memory (args != NULL), then the string "*rest" has some
finite length and won't cause strcat() to "go off into the weeds".
Or did you already observe that? I don't have a better solution.
If this is a buffer overrun/security issue, how could someone
exploit it?
-Anthony
> -----Original Message-----
> From: Charles Wilson [mailto:cygwin AT cwilson DOT fastmail DOT fm]
> Sent: Thursday, March 03, 2005 1:59
> To: cygwin AT cygwin DOT com
> Subject: Re: cygstart patch
>
>
> 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/
- Raw text -