httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <brian.p...@cnet.com>
Subject [PATCH] Re: mod_include bug(s)?
Date Thu, 28 Mar 2002 05:21:15 GMT
Here's a patch (against the current CVS head) that addresses the two
problems I know about:
  * The ctx->tag_length computation in find_end_sequence() was a bit
    broken in cases where there was a "false alarm" match on a partial
    "-->"
  * The ap_ssi_get_tag_and_value() function needs to avoid walking off
    the end of the string.  After debugging this some more, I ended up
    using Cliff's original patch.

With this patch, both the current CVS head and the bucket_allocator-patched
httpd pass all but one of the mod_include tests in httpd-test.  The test 
that
reports a failure is the if6.shtml one.  It's expecting three instances of
"[an error occurred..." but only two are output.  I believe the third one
that the test expects is due to an imbalanced "<!--#endif-->".  But based
on the code, I wouldn't expect to see the "[an error occurred..." message
for this situation.

Cliff and Paul (and anyone else with good test cases for this stuff), can
you test this patch against your test cases?

Thanks,
--Brian


Paul J. Reder wrote:

> Brian,
>
> Please give me a chance to fix this. I indicated that I was looking
> at this problem. There is no reason to duplicate work. I have identified
> several problems and am working on fixes for them. I should have 
> something
> tested and ready by the end of day on Thursday or Friday during the day
> at the latest.
>
> Paul J. Reder
>
>
> Paul J. Reder wrote:
>
>> Okay, I have recreated at least two problems in include processing, one
>> of which results in a core dump. I am in process of tracking them down.
>> It might be tomorrow before I have a patch.
>>
>> Paul J. Reder
>>
>> Paul J. Reder wrote:
>>
>>> Brian,
>>>
>>> I'm looking into this right now. I'll let you all know what I find out.
>>>
>>> I have some concerns about the suggested fix. I hope to have a fix
>>> by this afternoon.
>>>
>>> Paul J. Reder
>>>
>>> Brian Pane wrote:
>>>
>>>> Cliff Woolley wrote:
>>>>
>>>>> I've spent the entire evening chasing some wacky mod_include bugs 
>>>>> that
>>>>> surfaced as I was doing final testing on the bucket API patch.  At 
>>>>> first I
>>>>> assumed they were my fault, but upon further investigation I think 
>>>>> the
>>>>> fact that they haven't surfaced until now is a coincidence.  There 
>>>>> are two
>>>>> problems that I can see -- the if6.shtml and if7.shtml files I 
>>>>> committed
>>>>> to httpd-test last week to check for the mod_include 1.3 bug has 
>>>>> turned up
>>>>> quasi-related problems in mod_include 2.0 as well.
>>>>>
>>>>> Problem 1:
>>>>>
>>>>> When in an #if or #elif or several other kinds of tags,
>>>>> ap_ssi_get_tag_and_value() is called from within a while(1) loop that
>>>>> continues until that function returns with tag==NULL.  But in the 
>>>>> case of
>>>>> if6.shtml, ap_ssi_get_tag_and_value() steps right past the end of the
>>>>> buffer and never bothers to check and see how long the thing it's 
>>>>> supposed
>>>>> to be processing actually is.  The following patch fixes it, but 
>>>>> there
>>>>> could be a better way to do it.  I'm hoping someone out there who 
>>>>> knows
>>>>> this code better will come up with a better way to do it.
>>>>>
>>>>> Index: mod_include.c
>>>>> ===================================================================
>>>>> RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.c,v
>>>>> retrieving revision 1.205
>>>>> diff -u -d -r1.205 mod_include.c
>>>>> --- mod_include.c       24 Mar 2002 06:42:14 -0000      1.205
>>>>> +++ mod_include.c       27 Mar 2002 06:41:55 -0000
>>>>> @@ -866,6 +866,11 @@
>>>>>     int   shift_val = 0;
>>>>>     char  term = '\0';
>>>>>
>>>>> +    if (ctx->curr_tag_pos - ctx->combined_tag > ctx->tag_length)
{
>>>>> +        *tag = NULL;
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>>
>>>>
>>>> My only objection to this is that ctx->curr_tag_pos is supposed
>>>> to point to a null-terminated copy of the directive, and all the
>>>> subsequent looping logic in ap_ssi_tag_and_value() depends on
>>>> that.  Are we hitting a case where this string isn't null-terminated
>>>> (meaning that the root cause of the problem is somewhere else)?
>>>>
>>>>
>>>>>
>>>>>     *tag_val = NULL;
>>>>>     SKIP_TAG_WHITESPACE(c);
>>>>>     *tag = c;             /* First non-whitespace character (could 
>>>>> be NULL). */
>>>>>
>>>>>
>>>>> Problem 2:
>>>>>
>>>>> In the case of if7.shtml, for some reason, the null-terminating 
>>>>> character
>>>>> is placed one character too far forward.  So instead of #endif you 
>>>>> get
>>>>> #endif\b or some such garbage:
>>>>>
>>>> ...
>>>>
>>>>> Note the trailing \b in curr_tag_pos that shouldn't be there.
>>>>>
>>>>> I'm at a bit of a loss on this one.  I would think the problem 
>>>>> must be in
>>>>> get_combined_directive(), but I could be wrong.  Again, more eyes 
>>>>> would be
>>>>> appreciated.
>>>>>
>>>>
>>>> I'm willing to take a look at this later today.  The only problem
>>>> is that I can't recreate this problem (or the first one) with the
>>>> latest CVS head of httpd-test and httpd-2.0.  Is there any special
>>>> configuration needed to trigger the bug?
>>>>
>>>> --Brian
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>



Mime
View raw message