Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Subject: RE: cygstart patch
Date: Thu, 3 Mar 2005 08:47:57 -0500
Message-ID: <49D88D820A7BC0479A7B0932D4219EFE1DD0ED@NAEAPAXREX04VA.nadsusea.nads.navy.mil>
From: "Derosa, Anthony  CIV NAVAIR 2035, 2, 205/214" <Anthony DOT Derosa1 AT navy DOT mil>
To: <cygwin AT cygwin DOT com>
Content-Transfer-Encoding: 8bit

>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/