Bug 29455 - Patch for questionable http strtol use
Summary: Patch for questionable http strtol use
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.0.49
Hardware: All All
: P3 minor (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
: 33599 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-06-09 04:32 UTC by John Wehle
Modified: 2005-02-16 01:13 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John Wehle 2004-06-09 04:32:47 UTC
Code inspection shows strtol being used to convert "Content-Length"
from ASCII to a long which is then assigned to a apr_off_t variable.
This appears questionable since on some platforms (i.e. FreeBSD)
apr_off_t is larger than a long (which implies that "Content-Length"
may contain values which exceed a long).  This patch uses strtoll
in place of strtol.  It's been tested on FreeBSD 4.10 x86.

------------------8<------------------------8<------------------------
*** modules/http/http_protocol.c.ORIGINAL       Mon Feb  9 15:53:18 2004
--- modules/http/http_protocol.c        Tue Jun  8 03:30:07 2004
*************** apr_status_t ap_http_filter(ap_filter_t 
*** 769,775 ****
  
              ctx->state = BODY_LENGTH;
              errno = 0;
!             ctx->remaining = strtol(lenp, &endstr, 10);       /* we depend on 
ANSI */
  
              /* This protects us from over/underflow (the errno check),
               * non-digit chars in the string (excluding leading space)
--- 769,775 ----
  
              ctx->state = BODY_LENGTH;
              errno = 0;
!             ctx->remaining = (apr_off_t)strtoll(lenp, &endstr, 10); /* we depe
nd on ANSI */
  
              /* This protects us from over/underflow (the errno check),
               * non-digit chars in the string (excluding leading space)
*************** AP_DECLARE(int) ap_setup_client_block(re
*** 1756,1762 ****
          char *endstr;
  
          errno = 0;
!         r->remaining = strtol(lenp, &endstr, 10); /* depend on ANSI */
  
          /* See comments in ap_http_filter() */
          if (errno || (endstr && *endstr) || (r->remaining < 0)) {
--- 1756,1762 ----
          char *endstr;
  
          errno = 0;
!         r->remaining = (apr_off_t)strtoll(lenp, &endstr, 10); /* depend on ANS
I */
  
          /* See comments in ap_http_filter() */
          if (errno || (endstr && *endstr) || (r->remaining < 0)) {
Comment 1 André Malo 2004-06-09 08:39:01 UTC
That way it's not ready for common use, because apr_off_t is not always 64 bit.
In  2.1 there's a APR function for this (apr_strtoff), we might need to backport
or (perhaps better) implement the detection in httpd 2.0.
Comment 2 Joe Orton 2004-06-09 12:13:26 UTC
Yeah, backporting apr_strtoff to APR 0.9 would work only if it were used inside
#if APR_PATCH_VERSION > 4 or something, a little ugly but probably worth it.
Comment 3 André Malo 2004-06-09 12:22:01 UTC
Alternatively we could build in a runtime check or the like...?
Comment 4 Joe Orton 2004-06-09 12:32:30 UTC
Not sure what could be done at run-time.  If apr_strtoff is used but not defined
in libapr, that'll cause link failures.  A compile-time check, e.g.

if (sizeof(apr_off_t) == sizeof(long))
   foo = strtol(...);
else
   foo = apr_strtoi64(...)

works pretty much the same.
Comment 5 André Malo 2004-06-09 12:36:12 UTC
Ok, that's what I've meant as runtime check. Actually it's a mix out of compile
time (sizeof) and runtime stuff (if). Probably the optimizer does the rest ;)
Comment 6 Joe Orton 2005-01-17 16:15:46 UTC
Leaving as fixed for 2.1.x, there would be quite a bit to backport to get this
working for 2.0...
Comment 7 Joe Orton 2005-02-16 10:13:46 UTC
*** Bug 33599 has been marked as a duplicate of this bug. ***