You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2009/07/02 15:41:18 UTC

svn commit: r790587 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c

Author: jorton
Date: Thu Jul  2 13:41:18 2009
New Revision: 790587

URL: http://svn.apache.org/viewvc?rev=790587&view=rev
Log:
Security fix for CVE-2009-1890:

* modules/proxy/mod_proxy_http.c (stream_reqbody_cl): Specify the base
  passed to apr_strtoff, and validate the Content-Length in the same
  way the HTTP_IN filter does.  If the number of bytes streamed
  exceeds the expected body length, bail out of the loop.

Submitted by: niq, jorton

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/proxy/mod_proxy_http.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=790587&r1=790586&r2=790587&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Jul  2 13:41:18 2009
@@ -2,6 +2,11 @@
 
 Changes with Apache 2.3.3
 
+  *) SECURITY: CVE-2009-1890 (cve.mitre.org) 
+     Fix a potential Denial-of-Service attack against mod_proxy in a
+     reverse proxy configuration, where a remote attacker can force a
+     proxy process to consume CPU time indefinitely.  [Nick Kew, Joe Orton]
+
   *) SECURITY: CVE-2009-1191 (cve.mitre.org)
      mod_proxy_ajp: Avoid delivering content from a previous request which
      failed to send a request body. PR 46949 [Ruediger Pluem]

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=790587&r1=790586&r2=790587&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Thu Jul  2 13:41:18 2009
@@ -427,10 +427,16 @@
     apr_off_t bytes_streamed = 0;
 
     if (old_cl_val) {
+        char *endstr;
+
         add_cl(p, bucket_alloc, header_brigade, old_cl_val);
-        if (APR_SUCCESS != (status = apr_strtoff(&cl_val, old_cl_val, NULL,
-                                                 0))) {
-            return HTTP_INTERNAL_SERVER_ERROR;
+        status = apr_strtoff(&cl_val, old_cl_val, &endstr, 10);
+        
+        if (status || *endstr || endstr == old_cl_val || cl_val < 0) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
+                          "proxy: could not parse request Content-Length (%s)",
+                          old_cl_val);
+            return HTTP_BAD_REQUEST;
         }
     }
     terminate_headers(bucket_alloc, header_brigade);
@@ -463,8 +469,13 @@
          *
          * Prevents HTTP Response Splitting.
          */
-        if (bytes_streamed > cl_val)
-             continue;
+        if (bytes_streamed > cl_val) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                          "proxy: read more bytes of request body than expected "
+                          "(got %" APR_OFF_T_FMT ", expected %" APR_OFF_T_FMT ")",
+                          bytes_streamed, cl_val);
+            return HTTP_INTERNAL_SERVER_ERROR;
+        }
 
         if (header_brigade) {
             /* we never sent the header brigade, so go ahead and



Re: svn commit: r790587 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c

Posted by Joe Orton <jo...@redhat.com>.
On Sat, Jul 04, 2009 at 11:23:46PM +0100, Nick Kew wrote:
> jorton@apache.org wrote:
>
>>  Changes with Apache 2.3.3
>>  +  *) SECURITY: CVE-2009-1890 (cve.mitre.org) +     Fix a potential 
>> Denial-of-Service attack against mod_proxy in a
>> +     reverse proxy configuration, where a remote attacker can force a
>> +     proxy process to consume CPU time indefinitely.  [Nick Kew, Joe Orton]
>
> I thought in this instance, the original reporter's diagnostic
> work contributed more to the patch than we did.  I think he
> should be credited in the changelog here.

Lots of people help out with diagnosis of many bugs, we typically credit 
in CHANGES only those who came up with the patches.  I certainly should 
have given credit to the reporter in the commit message though, I will 
fix that.

Regards, Joe

Re: svn commit: r790587 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Nick Kew wrote:
> jorton@apache.org wrote:
> 
>>  Changes with Apache 2.3.3
>>  
>> +  *) SECURITY: CVE-2009-1890 (cve.mitre.org) +     Fix a potential
>> Denial-of-Service attack against mod_proxy in a
>> +     reverse proxy configuration, where a remote attacker can force a
>> +     proxy process to consume CPU time indefinitely.  [Nick Kew, Joe
>> Orton]
> 
> I thought in this instance, the original reporter's diagnostic
> work contributed more to the patch than we did.  I think he
> should be credited in the changelog here.

+1, and absolutely first credit, he nailed the bug on nose :)

Re: svn commit: r790587 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c

Posted by Nick Kew <ni...@webthing.com>.
jorton@apache.org wrote:

>  Changes with Apache 2.3.3
>  
> +  *) SECURITY: CVE-2009-1890 (cve.mitre.org) 
> +     Fix a potential Denial-of-Service attack against mod_proxy in a
> +     reverse proxy configuration, where a remote attacker can force a
> +     proxy process to consume CPU time indefinitely.  [Nick Kew, Joe Orton]

I thought in this instance, the original reporter's diagnostic
work contributed more to the patch than we did.  I think he
should be credited in the changelog here.

-- 
Nick Kew