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 23:02:16 GMT


On 02/25/2008 11:40 PM, Ruediger Pluem wrote:
> 
> 
> 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.

Ah. The following patch fixes this:

Index: modules/ssl/ssl_scache.c
===================================================================
--- modules/ssl/ssl_scache.c    (Revision 631020)
+++ modules/ssl/ssl_scache.c    (Arbeitskopie)
@@ -143,7 +143,7 @@
  {
      SSLModConfigRec *mc = myModConfig(r->server);

-    if (mc == NULL || flags & AP_STATUS_SHORT)
+    if (mc == NULL || flags & AP_STATUS_SHORT || mc->sesscache == NULL)
          return OK;

      ap_rputs("<hr>\n", r);


Regards

RĂ¼diger

Mime
View raw message