httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r630974 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_config.c ssl_private.h ssl_scache.c ssl_scache_dbm.c ssl_scache_dc.c ssl_scache_memcache.c ssl_scache_shmcb.c
Date Mon, 25 Feb 2008 22:40:56 GMT


On 02/25/2008 11:22 PM, Ruediger Pluem wrote:
> 
> 
> On 02/25/2008 10:45 PM, Joe Orton wrote:
>> On Mon, Feb 25, 2008 at 09:49:55PM +0100, Ruediger Pluem wrote:
>>> On 02/25/2008 09:09 PM, jorton@apache.org wrote:
>>>> Author: jorton
>>>> Date: Mon Feb 25 12:09:38 2008
>>>> New Revision: 630974
> 
>>>> -    if (mc->szSessionCacheDataFile == NULL) {
>>>> -        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "SSLSessionCache 
>>>> required");
>>>> -        return APR_EINVAL;
>>>> -    }
>>> Why don't we check any if arg == NULL as a replacement for the if 
>>> statement above?
>>
>> Sorry, I should have mentioned this in the changelog message.  In both 
>> cases of this, the code will always be passed a non-NULL arg parameter 
> 
> Hm, but you are supplying sep + 1 which is not NULL but can point to '\0'.
> Shouldn't we check for this?
> 
>> unless a pstrdup fails (which by policy will be ignored anyway).  I 
>> think these checks may have just been a copy'n'paste legacy from the 
>> shmcb code, which has genuine config argument parsing failure cases.
>>
>> Again, thanks for the detailed review!

Running the perl test suit I saw some regressions with core dumps:

t/modules/status.t                        1    1 100.00%  1
t/security/CVE-2006-5752.t                2    2 100.00%  1-2
t/security/CVE-2007-6388.t                2    2 100.00%  1-2

There might be something rotten in the mod_ssl status hook
now.

Regards

RĂ¼diger


Mime
View raw message