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 <[email protected]
<mailto:[email protected]>> 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/
