httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Plüm, Rüdiger, VF-Group" <ruediger.pl...@vodafone.com>
Subject RE: CVE-2011-3607, int overflow ap_pregsub()
Date Tue, 15 Nov 2011 14:22:30 GMT
The patch is fine on trunk because the affected code is not within  

 AP_DECLARE(char *) ap_pregsub(...)

but within

static apr_status_t regsub_core(apr_pool_t *p, char **result,
                                struct ap_varbuf *vb, const char *input,
                                const char *source, size_t nmatch,
                                ap_regmatch_t pmatch[], apr_size_t maxlen)

but there is no regsub_core in 2.2.x. So the patch needs to be adjusted for backport
to 2.2.x. But returning NULL in the 2.2.x case looks to be the correct thing to do
as this is how trunk behaves now.
OTOH there was some discussion on this list whether it is correct to backport this trunk
behaviour to 2.2.x.

Regards

Rüdiger

> -----Original Message-----
> From: Roman Drahtmueller [mailto:draht@suse.de] 
> Sent: Dienstag, 15. November 2011 15:13
> To: dev@httpd.apache.org
> Subject: CVE-2011-3607, int overflow ap_pregsub()
> 
> Hi there,
> 
> Revision 1198940 attempts to fix an integer overflow in 
> ap_pregsub() in 
> server/util.c:394. The patch is:
> 
> --- httpd/httpd/trunk/server/util.c	2011/11/07 21:09:41	1198939
> +++ httpd/httpd/trunk/server/util.c	2011/11/07 21:13:40	1198940
> @@ -411,6 +411,8 @@
>              len++;
>          }
>          else if (no < nmatch && pmatch[no].rm_so < 
> pmatch[no].rm_eo) {
> +            if (APR_SIZE_MAX - len <= pmatch[no].rm_eo - 
> pmatch[no].rm_so)
> +                return APR_ENOMEM;
>              len += pmatch[no].rm_eo - pmatch[no].rm_so;
>          }
> 
> 
> , and appears wrong, because, ap_pregsub() is
> 
> AP_DECLARE(char *) ap_pregsub(...)
> 
> This would require something along the lines of (proposal):
> 
> 
>          }
>          else if (no < nmatch && pmatch[no].rm_so < 
> pmatch[no].rm_eo) {
> +            if (APR_SIZE_MAX - len <= pmatch[no].rm_eo - 
> pmatch[no].rm_so) {
> +               ap_log_error(APLOG_MARK, APLOG_WARNING, 
> APR_ENOMEM, NULL,
> +                       "integer overflow or out of memory 
> condition." );
> +                return NULL;
> +           }
>              len += pmatch[no].rm_eo - pmatch[no].rm_so;
>          }
> 
>      }
> 
>      dest = dst = apr_pcalloc(p, len + 1);
> 
> +    if(!dest)
> +       return NULL;
> +
> +
>      /* Now actually fill in the string */
> 
> 
> ...or simply without the error logging.
> 
> Thoughts?
> Thanks,
> Roman.
> 

Mime
View raw message