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:dr...@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.
>
