httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Rumph <mike.ru...@oracle.com>
Subject Re: False positive errno test after call to strtol()
Date Fri, 15 Nov 2013 19:11:07 GMT
Thanks Jeff,

Here is an example with no validation:
modules/aaa/mod_auth_digest.c (lines 980 - 982):
============================================
     if (resp->opaque) {
         resp->opaque_num = (unsigned long) strtol(resp->opaque, NULL, 16);
     }

Here is an example with limited validation:
server/log.c (lines 1590 - 1600):
===========================
     /* If we fill the buffer, we're probably reading a corrupt pid file.
      * To be nice, let's also ensure the first char is a digit. */
     if (bytes_read == 0 || bytes_read == BUFFER_SIZE - 1 || 
!apr_isdigit(*buf)) {
         return APR_EGENERAL;
     }

     buf[bytes_read] = '\0';
     *mypid = strtol(buf, &endptr, 10);


Here is a typical example of endptr validation:
modules/proxy/mod_proxy_connect.c (lines 91 - 108):
===============================================
     const char *p = arg;

     if (!apr_isdigit(arg[0]))
         return "AllowCONNECT: port numbers must be numeric";

     first = strtol(p, &endptr, 10);
     if (*endptr == '-') {
         p = endptr + 1;
         last = strtol(p, &endptr, 10);
     }
     else {
         last = first;
     }

     if (endptr == p || *endptr != '\0')  {
         return apr_psprintf(parms->temp_pool,
                             "Cannot parse '%s' as port number", p);
     }

(There is no ERANGE checking for numeric overflow.)
(Also none of the calls to strtol() in Apache httpd and APR check for 
EINVAL.)

If I were to work on a more extensive patch, I would consider each case 
to see if further validation would be warranted.

Take care,

Mike


On 11/15/2013 9:38 AM, Jeff Trawick wrote:
> On Thu, Nov 14, 2013 at 4:11 PM, Mike Rumph <mike.rumph@oracle.com 
> <mailto:mike.rumph@oracle.com>> wrote:
>
>     The man page for strtol() indicate that the function can set errno
>     to ERANGE (EINVAL is also possible for some environments).
>     But for the errno check to be valid errno should be set to 0
>     before the function call.
>     - http://linux.die.net/man/3/strtol
>
>     I've reviewed all cases of calls to strtol() in httpd and APR code.
>     In some cases no validation is performed after the call.
>     In most cases endptr (the second parameter) is checked against the
>     beginning and/or ending of the string which does not guarantee
>     against numeric overflow.
>     In some cases errno is checked for ERANGE.
>
>     I've attached a patch for the simplest case, where errno is
>     checked but was not set to 0 before the call.
>
>
> committed to trunk as r1542338; I'll propose for backport to the 2.4.x 
> branch
>
>
>     I will consider working up a more extensive patch, if it is desired.
>
>
> I suggest posting a couple of examples of what you found first.
>
>
>     BTW, this discussion is not purely theoretical.
>     Erroneous "Invalid ThreadStackSize value: " messages have been
>     witnessed in HP-UX environments.
>
>     Thanks,
>
>     Mike Rumph
>
>
>
>
> -- 
> Born in Roswell... married an alien...
> http://emptyhammock.com/


Mime
View raw message