Hi,

AFAIK, no crash has ever been reported for that.
I just noted this while looking at PR56287 and found it odd.

A file such as:
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
<html><head>
<meta http-equiv="Conten" content><head><body></body></html>
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-

will trigger the scan past the end of the buffer behavior.

line 658: ap_regexec(<meta[^>]*(http-equiv)[^>]*>)     OK
line 665: strncasecmp(header, "Content-" OK, string not found
line 667: apr_strmatch("content"                          OK
then looping with +7 every time until reaching a \0 because no '=' is in the buffer

In the example above, we go past then end of 'buf' because of the unconditional +7 and no check that we are about to scan past the end of 'buf'

So, the ocde with a 'continue' can:
    - eat time until we find a \0 somewhere
- do a 'content = apr_pstrndup' with unexpected data if we finally reach a "=" (however, the strdup will not copy data past the end of buf)
    - crash if we try to read memory that does not belong to the process ?

So I imagine that someone who can write a file on the server behind the proxy could set up what looks like a DoS, expecting to crash a process from time to time.
All this is true if 'ProxyHTMLMeta' is on, which is not the default.


This is all theoretical and with my tests, I never managed to crash my server. Anyway, the logic that was in place was broken. My change is not perfect (we should try to find another 'content', if any, to keep the spirit of the code), but is, IMO, safer because avoid a possible crash.


Should you share my analysis and should a CHANGE be useful for what I think is a corner case, feel free to add something, or I can do it by the end of the week.


Best regards,
CJ




Le 15/04/2014 21:16, Jeff Trawick a écrit :
On Fri, Apr 4, 2014 at 4:30 PM, <jaillet...@apache.org <mailto:jaillet...@apache.org>> wrote:

    Author: jailletc36
    Date: Fri Apr  4 20:30:38 2014
    New Revision: 1584896

    URL: http://svn.apache.org/r1584896
    Log:
    Do not perform a p+= 7 that could go past the end of the buffer in
    case we find a 'content' without a corresponding '='.


Does this fix a crash or a parsing error or ...?  (CHANGES)


    Should we need to deal with this case, a new search should be
    performed to find the real starting position of another potential
    'content=' pattern.

    Modified:
        httpd/httpd/trunk/modules/filters/mod_proxy_html.c

    Modified: httpd/httpd/trunk/modules/filters/mod_proxy_html.c
    URL:
    
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_proxy_html.c?rev=1584896&r1=1584895&r2=1584896&view=diff
    
==============================================================================
    --- httpd/httpd/trunk/modules/filters/mod_proxy_html.c (original)
    +++ httpd/httpd/trunk/modules/filters/mod_proxy_html.c Fri Apr  4
    20:30:38 2014
    @@ -672,8 +672,9 @@ static meta *metafix(request_rec *r, con
                         p += 7;
                         while (apr_isspace(*p))
                             ++p;
    +                    /* XXX Should we search for another content=
    pattern? */
                         if (*p != '=')
    -                        continue;
    +                        break;
                         while (*p && apr_isspace(*++p));
                         if ((*p == '\'') || (*p == '"')) {
                             delim = *p++;





--
Born in Roswell... married an alien...
http://emptyhammock.com/
http://edjective.org/


Reply via email to