You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Randy Terbush <ra...@zyzzyva.com> on 1997/03/02 19:03:33 UTC
Re: [BUG]: "segv attempting to run some cgis" on OTHER:qnx
> > > Symptoms:
> > > --
> > > httpd faults. the bug is that va_end() is called
> > > twice within create_argv() [in util_script.c]
> > > va_end() should not be called at the bottom of
> > > the while loop.
> > > --
> >
> > Hi,
> >
> > Thanks for the info. It'll be looked into for the next
> > beta.
>
> This also fixes another PR somewhere in the database.
>
> Ok, there are a few problems with create_argv:
>
> - possible buffer overflow if command line args >
> APACHE_ARG_MAX; not likely to be exploitable.
I don't see this Marc. Can you be more specific?
> - av[idx] = '\0' should be NULL instead of '\0'
?? array of char *?
> - remove the first va_end
Ya.
> - I didn't think that va_arg would necessarily work properly
> with a NULL (ie. the NULL termination), since NULL is
> generally 0 and 0 is an int not a char* so va_arg may not
> match it?
Hmm, I had thought that last I looked NULL was void *(0) or
something. You are correct though that it is 0 on FreeBSD. and BSDI.
> char **create_argv(request_rec *r, char *av0, ...)
> {
> int idx;
> char **av;
> char *t, *arg;
> va_list args;
>
> av = (char **)palloc(r->pool, APACHE_ARG_MAX);
>
> av[0] = av0;
> idx = 1;
>
> va_start(args, av0);
> while ((arg = va_arg(args, char *)) != NULL) {
> if ((t = strtok(arg, "+")) == NULL)
> break;
>
> unescape_url(t);
> av[idx] = escape_shell_cmd(r->pool, t);
> av[idx] = t;
> idx++;
> if (idx >= APACHE_ARG_MAX-1) break;
>
> while ((t = strtok(NULL, "+")) != NULL) {
> unescape_url(t);
> assert(idx < APACHE_ARG_MAX);
> av[idx] = escape_shell_cmd(r->pool, t);
> av[idx] = t;
> idx++;
> if (idx >= APACHE_ARG_MAX-1) break;
> }
> va_end(args);
> }
> va_end(args);
>
> av[idx] = '\0';
> return av;
> }
>
Re: [BUG]: "segv attempting to run some cgis" on OTHER:qnx
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 2 Mar 1997, Randy Terbush wrote:
> > - I didn't think that va_arg would necessarily work properly
> > with a NULL (ie. the NULL termination), since NULL is
> > generally 0 and 0 is an int not a char* so va_arg may not
> > match it?
>
> Hmm, I had thought that last I looked NULL was void *(0) or
> something. You are correct though that it is 0 on FreeBSD. and BSDI.
0 is a special integer. It's also a pointer of any type.
> > av[idx] = escape_shell_cmd(r->pool, t);
> > av[idx] = t;
Ok so I like just woke up, but this seems totally suspect to me. It
just tosses the result of escape_shell_cmd.
Dean
Re: [BUG]: "segv attempting to run some cgis" on OTHER:qnx
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 2 Mar 1997, Marc Slemko wrote:
> ...and AIX and Solaris and SunOS and IRIX. So should wherever we
> call create_argv, like:
>
> execv(r->filename, create_argv(r, argv0, r->args, NULL));
>
> be changed to something like:
>
> execv(r->filename, create_argv(r, argv0, r->args, (void *)NULL));
>
> ? It works as it is, but is that just the compiler being smarter than it
> has to? I know I have C books somewhere that warn against using NULL
> without casting in this situation...
Ah yes you're right, I forgot about the unprototyped case. This could
be a problem on QNX if they're running the 16-bit QNX in any of the
large data models where pointers are 32-bits and ints are 16-bits.
QNX uses WATCOM's C compiler, which I'm quite familiar with having
worked there... at any rate the def'n of NULL changed several times
over the years and I'm not sure if it's ((void*)0) or just (0) now.
The whole problem is compounded on the 286 by "near" and "far" pointers.
(And also on the 386, but everyone just uses the flat memory model with
near pointers.)
Dean
Re: [BUG]: "segv attempting to run some cgis" on OTHER:qnx
Posted by Marc Slemko <ma...@znep.com>.
On Sun, 2 Mar 1997, Randy Terbush wrote:
>
> > > > Symptoms:
> > > > --
> > > > httpd faults. the bug is that va_end() is called
> > > > twice within create_argv() [in util_script.c]
> > > > va_end() should not be called at the bottom of
> > > > the while loop.
> > > > --
> > >
> > > Hi,
> > >
> > > Thanks for the info. It'll be looked into for the next
> > > beta.
> >
> > This also fixes another PR somewhere in the database.
> >
> > Ok, there are a few problems with create_argv:
> >
> > - possible buffer overflow if command line args >
> > APACHE_ARG_MAX; not likely to be exploitable.
>
> I don't see this Marc. Can you be more specific?
Sorry, I'm crazy.
>
> > - av[idx] = '\0' should be NULL instead of '\0'
>
> ?? array of char *?
av is a **char. av[idx] is a *char. A pointer is not '\0', it is NULL.
If you would not say av[idx] = 'a' you should not say av[idx] = '\0'.
No?
>
> > - remove the first va_end
>
> Ya.
>
> > - I didn't think that va_arg would necessarily work properly
> > with a NULL (ie. the NULL termination), since NULL is
> > generally 0 and 0 is an int not a char* so va_arg may not
> > match it?
>
> Hmm, I had thought that last I looked NULL was void *(0) or
> something. You are correct though that it is 0 on FreeBSD. and BSDI.
...and AIX and Solaris and SunOS and IRIX. So should wherever we
call create_argv, like:
execv(r->filename, create_argv(r, argv0, r->args, NULL));
be changed to something like:
execv(r->filename, create_argv(r, argv0, r->args, (void *)NULL));
? It works as it is, but is that just the compiler being smarter than it
has to? I know I have C books somewhere that warn against using NULL
without casting in this situation...
>
>
> > char **create_argv(request_rec *r, char *av0, ...)
> > {
> > int idx;
> > char **av;
> > char *t, *arg;
> > va_list args;
> >
> > av = (char **)palloc(r->pool, APACHE_ARG_MAX);
> >
> > av[0] = av0;
> > idx = 1;
> >
> > va_start(args, av0);
> > while ((arg = va_arg(args, char *)) != NULL) {
> > if ((t = strtok(arg, "+")) == NULL)
> > break;
> >
> > unescape_url(t);
> > av[idx] = escape_shell_cmd(r->pool, t);
> > av[idx] = t;
> > idx++;
> > if (idx >= APACHE_ARG_MAX-1) break;
> >
> > while ((t = strtok(NULL, "+")) != NULL) {
> > unescape_url(t);
> > assert(idx < APACHE_ARG_MAX);
> > av[idx] = escape_shell_cmd(r->pool, t);
> > av[idx] = t;
> > idx++;
> > if (idx >= APACHE_ARG_MAX-1) break;
> > }
> > va_end(args);
> > }
> > va_end(args);
> >
> > av[idx] = '\0';
> > return av;
> > }
> >
>
>
>