We should come to a conclusion on this.
On 11/15/2011 8:22 AM, "Plüm, Rüdiger, VF-Group" wrote:
> 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:[email protected]]
>> Sent: Dienstag, 15. November 2011 15:13
>> To: [email protected]
>> 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.
>>
>
