You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2007/08/06 19:27:11 UTC

svn commit: r563198 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/proxy/proxy_util.c

Author: covener
Date: Mon Aug  6 10:27:09 2007
New Revision: 563198

URL: http://svn.apache.org/viewvc?view=rev&rev=563198
Log:
backport mod_proxy date parsing buffer over-read

Submitted by: Nick Kew, Davi Arnaut
Reviewed by: niq, rpluem, covener

Modified:
    httpd/httpd/branches/2.2.x/CHANGES
    httpd/httpd/branches/2.2.x/STATUS
    httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c

Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?view=diff&rev=563198&r1=563197&r2=563198
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Mon Aug  6 10:27:09 2007
@@ -1,6 +1,11 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.2.5
 
+  *) SECURITY: CVE-2007-3847 (cve.mitre.org)
+     mod_proxy: Prevent reading past the end of a buffer when parsing
+     date-related headers.  PR 41144.
+     [Nick Kew, Davi Arnaut]
+    
   *) SECURITY: CVE-2007-1863 (cve.mitre.org)
      mod_cache: Prevent a segmentation fault if attributes are listed in a 
      Cache-Control header without any value. 

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?view=diff&rev=563198&r1=563197&r2=563198
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Mon Aug  6 10:27:09 2007
@@ -329,15 +329,6 @@
       2.2.x: trunk will work once PR: 23287 patch is applied.
       +1: niq
 
-    * mod_proxy: fix buffer overread in date parsing
-      PR: 41144
-      CVE-2007-3847
-      Trunk: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?r1=550514&r2=562069
-      2.2.x: http://people.apache.org/~niq/proxy-util-22x.patch
-      +1: niq
-      rpluem says: Only one minor style nit: There is a tab before
-      ap_proxy_date_canon. Otherwise +1 on the patch.
-
     * mod_core: Avoid that relative changes to Options change the settings for
       FileETag. This does NOT address the remaining issues with relative
       settings and FileETag mentioned in PR 42027, but at least it isolates the

Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?view=diff&rev=563198&r1=563197&r2=563198
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Mon Aug  6 10:27:09 2007
@@ -280,70 +280,28 @@
     return NULL;
 }
 
-static const char * const lwday[7] =
-{"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};
-
 /*
  * If the date is a valid RFC 850 date or asctime() date, then it
- * is converted to the RFC 1123 format, otherwise it is not modified.
- * This routine is not very fast at doing conversions, as it uses
- * sscanf and sprintf. However, if the date is already correctly
- * formatted, then it exits very quickly.
+ * is converted to the RFC 1123 format.
  */
 PROXY_DECLARE(const char *)
-     ap_proxy_date_canon(apr_pool_t *p, const char *x1)
+     ap_proxy_date_canon(apr_pool_t *p, const char *date)
 {
-    char *x = apr_pstrdup(p, x1);
-    int wk, mday, year, hour, min, sec, mon;
-    char *q, month[4], zone[4], week[4];
+    apr_status_t rv;
+    char* ndate;
 
-    q = strchr(x, ',');
-    /* check for RFC 850 date */
-    if (q != NULL && q - x > 3 && q[1] == ' ') {
-    *q = '\0';
-    for (wk = 0; wk < 7; wk++)
-        if (strcmp(x, lwday[wk]) == 0)
-        break;
-    *q = ',';
-    if (wk == 7)
-        return x;       /* not a valid date */
-    if (q[4] != '-' || q[8] != '-' || q[11] != ' ' || q[14] != ':' ||
-        q[17] != ':' || strcmp(&q[20], " GMT") != 0)
-        return x;
-    if (sscanf(q + 2, "%u-%3s-%u %u:%u:%u %3s", &mday, month, &year,
-           &hour, &min, &sec, zone) != 7)
-        return x;
-    if (year < 70)
-        year += 2000;
-    else
-        year += 1900;
-    }
-    else {
-/* check for acstime() date */
-    if (x[3] != ' ' || x[7] != ' ' || x[10] != ' ' || x[13] != ':' ||
-        x[16] != ':' || x[19] != ' ' || x[24] != '\0')
-        return x;
-    if (sscanf(x, "%3s %3s %u %u:%u:%u %u", week, month, &mday, &hour,
-           &min, &sec, &year) != 7)
-        return x;
-    for (wk = 0; wk < 7; wk++)
-        if (strcmp(week, apr_day_snames[wk]) == 0)
-        break;
-    if (wk == 7)
-        return x;
+    apr_time_t time = apr_date_parse_http(date);
+    if (!time) {
+        return date;
     }
 
-/* check date */
-    for (mon = 0; mon < 12; mon++)
-    if (strcmp(month, apr_month_snames[mon]) == 0)
-        break;
-    if (mon == 12)
-    return x;
+    ndate = apr_palloc(p, APR_RFC822_DATE_LEN);
+    rv = apr_rfc822_date(ndate, time);
+    if (rv != APR_SUCCESS) {
+        return date;
+    }
 
-    q = apr_palloc(p, 30);
-    apr_snprintf(q, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT", apr_day_snames[wk],
-       mday, apr_month_snames[mon], year, hour, min, sec);
-    return q;
+    return ndate;
 }
 
 PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)



Re: svn commit: r563198 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS

Posted by Jim Jagielski <ji...@jaguNET.com>.
Umm... It's usually best to actually do this according
to normal process:

  1. Cast your vote
  2. Then promote as Accepted
  3. Then apply
  4. Then clean up.

covener@apache.org wrote:
> 
> Author: covener
> Date: Mon Aug  6 10:27:09 2007
> New Revision: 563198
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=563198
> Log:
> backport mod_proxy date parsing buffer over-read
> 
> Submitted by: Nick Kew, Davi Arnaut
> Reviewed by: niq, rpluem, covener
> 
> Modified:
>     httpd/httpd/branches/2.2.x/CHANGES
>     httpd/httpd/branches/2.2.x/STATUS
>     httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> 
> Modified: httpd/httpd/branches/2.2.x/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?view=diff&rev=563198&r1=563197&r2=563198
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
> +++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Mon Aug  6 10:27:09 2007
> @@ -1,6 +1,11 @@
>                                                          -*- coding: utf-8 -*-
>  Changes with Apache 2.2.5
>  
> +  *) SECURITY: CVE-2007-3847 (cve.mitre.org)
> +     mod_proxy: Prevent reading past the end of a buffer when parsing
> +     date-related headers.  PR 41144.
> +     [Nick Kew, Davi Arnaut]
> +    
>    *) SECURITY: CVE-2007-1863 (cve.mitre.org)
>       mod_cache: Prevent a segmentation fault if attributes are listed in a 
>       Cache-Control header without any value. 
> 
> Modified: httpd/httpd/branches/2.2.x/STATUS
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?view=diff&rev=563198&r1=563197&r2=563198
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Mon Aug  6 10:27:09 2007
> @@ -329,15 +329,6 @@
>        2.2.x: trunk will work once PR: 23287 patch is applied.
>        +1: niq
>  
> -    * mod_proxy: fix buffer overread in date parsing
> -      PR: 41144
> -      CVE-2007-3847
> -      Trunk: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?r1=550514&r2=562069
> -      2.2.x: http://people.apache.org/~niq/proxy-util-22x.patch
> -      +1: niq
> -      rpluem says: Only one minor style nit: There is a tab before
> -      ap_proxy_date_canon. Otherwise +1 on the patch.
> -
>      * mod_core: Avoid that relative changes to Options change the settings for
>        FileETag. This does NOT address the remaining issues with relative
>        settings and FileETag mentioned in PR 42027, but at least it isolates the
> 
> Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?view=diff&rev=563198&r1=563197&r2=563198
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Mon Aug  6 10:27:09 2007
> @@ -280,70 +280,28 @@
>      return NULL;
>  }
>  
> -static const char * const lwday[7] =
> -{"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};
> -
>  /*
>   * If the date is a valid RFC 850 date or asctime() date, then it
> - * is converted to the RFC 1123 format, otherwise it is not modified.
> - * This routine is not very fast at doing conversions, as it uses
> - * sscanf and sprintf. However, if the date is already correctly
> - * formatted, then it exits very quickly.
> + * is converted to the RFC 1123 format.
>   */
>  PROXY_DECLARE(const char *)
> -     ap_proxy_date_canon(apr_pool_t *p, const char *x1)
> +     ap_proxy_date_canon(apr_pool_t *p, const char *date)
>  {
> -    char *x = apr_pstrdup(p, x1);
> -    int wk, mday, year, hour, min, sec, mon;
> -    char *q, month[4], zone[4], week[4];
> +    apr_status_t rv;
> +    char* ndate;
>  
> -    q = strchr(x, ',');
> -    /* check for RFC 850 date */
> -    if (q != NULL && q - x > 3 && q[1] == ' ') {
> -    *q = '\0';
> -    for (wk = 0; wk < 7; wk++)
> -        if (strcmp(x, lwday[wk]) == 0)
> -        break;
> -    *q = ',';
> -    if (wk == 7)
> -        return x;       /* not a valid date */
> -    if (q[4] != '-' || q[8] != '-' || q[11] != ' ' || q[14] != ':' ||
> -        q[17] != ':' || strcmp(&q[20], " GMT") != 0)
> -        return x;
> -    if (sscanf(q + 2, "%u-%3s-%u %u:%u:%u %3s", &mday, month, &year,
> -           &hour, &min, &sec, zone) != 7)
> -        return x;
> -    if (year < 70)
> -        year += 2000;
> -    else
> -        year += 1900;
> -    }
> -    else {
> -/* check for acstime() date */
> -    if (x[3] != ' ' || x[7] != ' ' || x[10] != ' ' || x[13] != ':' ||
> -        x[16] != ':' || x[19] != ' ' || x[24] != '\0')
> -        return x;
> -    if (sscanf(x, "%3s %3s %u %u:%u:%u %u", week, month, &mday, &hour,
> -           &min, &sec, &year) != 7)
> -        return x;
> -    for (wk = 0; wk < 7; wk++)
> -        if (strcmp(week, apr_day_snames[wk]) == 0)
> -        break;
> -    if (wk == 7)
> -        return x;
> +    apr_time_t time = apr_date_parse_http(date);
> +    if (!time) {
> +        return date;
>      }
>  
> -/* check date */
> -    for (mon = 0; mon < 12; mon++)
> -    if (strcmp(month, apr_month_snames[mon]) == 0)
> -        break;
> -    if (mon == 12)
> -    return x;
> +    ndate = apr_palloc(p, APR_RFC822_DATE_LEN);
> +    rv = apr_rfc822_date(ndate, time);
> +    if (rv != APR_SUCCESS) {
> +        return date;
> +    }
>  
> -    q = apr_palloc(p, 30);
> -    apr_snprintf(q, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT", apr_day_snames[wk],
> -       mday, apr_month_snames[mon], year, hour, min, sec);
> -    return q;
> +    return ndate;
>  }
>  
>  PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
> 
> 


-- 
===========================================================================
   Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
	    "If you can dodge a wrench, you can dodge a ball."

Re: svn commit: r563198 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS

Posted by Jim Jagielski <ji...@jaguNET.com>.
Umm... It's usually best to actually do this according
to normal process:

  1. Cast your vote
  2. Then promote as Accepted
  3. Then apply
  4. Then clean up.

covener@apache.org wrote:
> 
> Author: covener
> Date: Mon Aug  6 10:27:09 2007
> New Revision: 563198
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=563198
> Log:
> backport mod_proxy date parsing buffer over-read
> 
> Submitted by: Nick Kew, Davi Arnaut
> Reviewed by: niq, rpluem, covener
> 
> Modified:
>     httpd/httpd/branches/2.2.x/CHANGES
>     httpd/httpd/branches/2.2.x/STATUS
>     httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> 
> Modified: httpd/httpd/branches/2.2.x/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?view=diff&rev=563198&r1=563197&r2=563198
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
> +++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Mon Aug  6 10:27:09 2007
> @@ -1,6 +1,11 @@
>                                                          -*- coding: utf-8 -*-
>  Changes with Apache 2.2.5
>  
> +  *) SECURITY: CVE-2007-3847 (cve.mitre.org)
> +     mod_proxy: Prevent reading past the end of a buffer when parsing
> +     date-related headers.  PR 41144.
> +     [Nick Kew, Davi Arnaut]
> +    
>    *) SECURITY: CVE-2007-1863 (cve.mitre.org)
>       mod_cache: Prevent a segmentation fault if attributes are listed in a 
>       Cache-Control header without any value. 
> 
> Modified: httpd/httpd/branches/2.2.x/STATUS
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?view=diff&rev=563198&r1=563197&r2=563198
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Mon Aug  6 10:27:09 2007
> @@ -329,15 +329,6 @@
>        2.2.x: trunk will work once PR: 23287 patch is applied.
>        +1: niq
>  
> -    * mod_proxy: fix buffer overread in date parsing
> -      PR: 41144
> -      CVE-2007-3847
> -      Trunk: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?r1=550514&r2=562069
> -      2.2.x: http://people.apache.org/~niq/proxy-util-22x.patch
> -      +1: niq
> -      rpluem says: Only one minor style nit: There is a tab before
> -      ap_proxy_date_canon. Otherwise +1 on the patch.
> -
>      * mod_core: Avoid that relative changes to Options change the settings for
>        FileETag. This does NOT address the remaining issues with relative
>        settings and FileETag mentioned in PR 42027, but at least it isolates the
> 
> Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?view=diff&rev=563198&r1=563197&r2=563198
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Mon Aug  6 10:27:09 2007
> @@ -280,70 +280,28 @@
>      return NULL;
>  }
>  
> -static const char * const lwday[7] =
> -{"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"};
> -
>  /*
>   * If the date is a valid RFC 850 date or asctime() date, then it
> - * is converted to the RFC 1123 format, otherwise it is not modified.
> - * This routine is not very fast at doing conversions, as it uses
> - * sscanf and sprintf. However, if the date is already correctly
> - * formatted, then it exits very quickly.
> + * is converted to the RFC 1123 format.
>   */
>  PROXY_DECLARE(const char *)
> -     ap_proxy_date_canon(apr_pool_t *p, const char *x1)
> +     ap_proxy_date_canon(apr_pool_t *p, const char *date)
>  {
> -    char *x = apr_pstrdup(p, x1);
> -    int wk, mday, year, hour, min, sec, mon;
> -    char *q, month[4], zone[4], week[4];
> +    apr_status_t rv;
> +    char* ndate;
>  
> -    q = strchr(x, ',');
> -    /* check for RFC 850 date */
> -    if (q != NULL && q - x > 3 && q[1] == ' ') {
> -    *q = '\0';
> -    for (wk = 0; wk < 7; wk++)
> -        if (strcmp(x, lwday[wk]) == 0)
> -        break;
> -    *q = ',';
> -    if (wk == 7)
> -        return x;       /* not a valid date */
> -    if (q[4] != '-' || q[8] != '-' || q[11] != ' ' || q[14] != ':' ||
> -        q[17] != ':' || strcmp(&q[20], " GMT") != 0)
> -        return x;
> -    if (sscanf(q + 2, "%u-%3s-%u %u:%u:%u %3s", &mday, month, &year,
> -           &hour, &min, &sec, zone) != 7)
> -        return x;
> -    if (year < 70)
> -        year += 2000;
> -    else
> -        year += 1900;
> -    }
> -    else {
> -/* check for acstime() date */
> -    if (x[3] != ' ' || x[7] != ' ' || x[10] != ' ' || x[13] != ':' ||
> -        x[16] != ':' || x[19] != ' ' || x[24] != '\0')
> -        return x;
> -    if (sscanf(x, "%3s %3s %u %u:%u:%u %u", week, month, &mday, &hour,
> -           &min, &sec, &year) != 7)
> -        return x;
> -    for (wk = 0; wk < 7; wk++)
> -        if (strcmp(week, apr_day_snames[wk]) == 0)
> -        break;
> -    if (wk == 7)
> -        return x;
> +    apr_time_t time = apr_date_parse_http(date);
> +    if (!time) {
> +        return date;
>      }
>  
> -/* check date */
> -    for (mon = 0; mon < 12; mon++)
> -    if (strcmp(month, apr_month_snames[mon]) == 0)
> -        break;
> -    if (mon == 12)
> -    return x;
> +    ndate = apr_palloc(p, APR_RFC822_DATE_LEN);
> +    rv = apr_rfc822_date(ndate, time);
> +    if (rv != APR_SUCCESS) {
> +        return date;
> +    }
>  
> -    q = apr_palloc(p, 30);
> -    apr_snprintf(q, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT", apr_day_snames[wk],
> -       mday, apr_month_snames[mon], year, hour, min, sec);
> -    return q;
> +    return ndate;
>  }
>  
>  PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
> 
> 


-- 
===========================================================================
   Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
	    "If you can dodge a wrench, you can dodge a ball."