You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2007/08/01 10:05:33 UTC
Re: svn commit: r561616 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
On Wed, Aug 01, 2007 at 12:58:21AM -0000, niq@apache.org wrote:
> Author: niq
> Date: Tue Jul 31 17:58:20 2007
> New Revision: 561616
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=561616
> Log:
> Fix buffer overflow in date handling
> PR 41144 (Davi Arnaut)
This appears to be a buffer "over-read", not a buffer overflow, correct?
This allows a malicious origin server to possibly cause a process crash
on a caching forward proxy, which is a DoS for a threaded MPM, so it
should be treated as a security issue and needs a CVE name.
Mark has assigned CVE-2007-3847.
(Davi, we encourage people to report security issues to
security@httpd.apache.org rather than using bugzilla, FWIW)
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?view=diff&rev=561616&r1=561615&r2=561616
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Tue Jul 31 17:58:20 2007
> @@ -2,6 +2,9 @@
> Changes with Apache 2.3.0
> [Remove entries to the current 2.0 and 2.2 section below, when backported]
>
> + *) mod_proxy: fix buffer overflow issue
> + PR 41144 [Davi Arnaut, Nick Kew]
> +
> *) mod_deflate: fix protocol handling in deflate input filter
> PR 23287 [Nick Kew]
>
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?view=diff&rev=561616&r1=561615&r2=561616
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Jul 31 17:58:20 2007
> @@ -306,85 +306,37 @@
> 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];
> -
> - 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_status_t rv;
> + apr_time_exp_t tm;
> + apr_size_t retsize;
> + char* ndate;
> + static const char format[] = "%a, %d %b %Y %H:%M:%S GMT";
> + 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;
> - }
> + rv = apr_time_exp_gmt(&tm, time);
> +
> + if (rv != APR_SUCCESS) {
> + return date;
> }
> - if (mon == 12) {
> - return x;
> +
> + ndate = apr_palloc(p, APR_RFC822_DATE_LEN);
> + rv = apr_strftime(ndate, &retsize, APR_RFC822_DATE_LEN, format, &tm);
> +
> + if (rv != APR_SUCCESS || !retsize) {
> + 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: r561616 - in /httpd/httpd/trunk: CHANGES
modules/proxy/proxy_util.c
Posted by Nick Kew <ni...@webthing.com>.
On Wed, 1 Aug 2007 11:10:19 +0200
Plüm, Rüdiger, VF-Group <ru...@vodafone.com> wrote:
> Why not using apr_rfc822_date instead? This is makes this function
> even shorter and from a first glance apr_rfc822_date is far more
> efficient then apr_strftime.
Indeedie. I have no idea why not. The patch was an old one
that's been languishing in bugzilla.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Re: svn commit: r561616 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Plüm,
Rüdiger,
VF-Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Joe Orton
> Gesendet: Mittwoch, 1. August 2007 10:06
> An: dev@httpd.apache.org
> Cc: Davi Arnaut
> Betreff: Re: svn commit: r561616 - in /httpd/httpd/trunk:
> CHANGES modules/proxy/proxy_util.c
>
>
> On Wed, Aug 01, 2007 at 12:58:21AM -0000, wrote:
> > Author: niq
> > Date: Tue Jul 31 17:58:20 2007
> > New Revision: 561616
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=561616
> > Log:
> > Fix buffer overflow in date handling
> > PR 41144 (Davi Arnaut)
>
> This appears to be a buffer "over-read", not a buffer
> overflow, correct?
I agree with this.
> >
> ==============================================================
> ================
> > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue Jul 31
> > - }
> > + apr_status_t rv;
> > + apr_time_exp_t tm;
> > + apr_size_t retsize;
> > + char* ndate;
> > + static const char format[] = "%a, %d %b %Y %H:%M:%S GMT";
> > + 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;
> > - }
> > + rv = apr_time_exp_gmt(&tm, time);
> > +
> > + if (rv != APR_SUCCESS) {
> > + return date;
> > }
> > - if (mon == 12) {
> > - return x;
> > +
> > + ndate = apr_palloc(p, APR_RFC822_DATE_LEN);
> > + rv = apr_strftime(ndate, &retsize,
> APR_RFC822_DATE_LEN, format, &tm);
Why not using apr_rfc822_date instead? This is makes this function even shorter
and from a first glance apr_rfc822_date is far more efficient then apr_strftime.
Regards
Rüdiger