tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject [Bug 62459] mod_jk: Forwarding URLs containing escaped slashes (e.g. for REST services) fail with syntactical-wrong double-escaping
Date Thu, 23 Aug 2018 09:06:41 GMT
https://bz.apache.org/bugzilla/show_bug.cgi?id=62459

--- Comment #16 from Guido Jäkel <G.Jaekel@DNB.DE> ---
(In reply to Mark Thomas from comment #15)
> Please stop changing the resolution of this issue. The correct resolution is
> WONTFIX.

Sorry, but I don't change it by intention.

(In reply to Guido Jäkel from comment #8)
> If you think (or even know), this CAN and is valid -- e.g. for some other
> part of the URL than the path elements section, THEN my suggestion may be
> extended to have the "syntactical knowledge" to act only on path elements.

I examine the code at  apache-2.0/mod_jk.c::init_ws_service()  , where 
commmon/jk_url.c::jk_canonenc()  is called: The stringbuffer passed here to
encode consist of the pure path section only, the other URI elements like the
query sting or host name part are separate. Therefore, there is no need to
implement a syntactical knowledge of the URI parts into  jk_cononenc()  at the
moment -- it will act on the path and a '%2F' appear here, it must be the
result of a "encoded slash". And this is either enabled by AllowEncodedSlashes
or rejected upstream before.

It is also used once just here (and in the same matter for iis and netscape)
and in the case that 

[...]
    case JK_OPT_FWDURICOMPATUNPARSED:
        s->req_uri = r->unparsed_uri;
        if (s->req_uri != NULL) {
            char *query_str = strchr(s->req_uri, '?');
            if (query_str != NULL) {
                *query_str = 0;
            }
        }

        break;

    case JK_OPT_FWDURICOMPAT:
        s->req_uri = r->uri;
        break;

    case JK_OPT_FWDURIPROXY:
        size = 3 * (int)strlen(r->uri) + 1;
        s->req_uri = apr_palloc(r->pool, size);
        jk_canonenc(r->uri, s->req_uri, size);       <------
        break;

    case JK_OPT_FWDURIESCAPED:
        s->req_uri = ap_escape_uri(r->pool, r->uri);
        break;

    default:
        return JK_FALSE;
    }
[...]

In this case, the '%2F' is intentional allowed (keeping in mind the potential
directory traversal issue of a buggy backend). And the task of this patch is to
prevent mod_jk from breaking this from '%2F' into '%252F' by replacing the
first percent char of by '%25'


A remaining question is: What's about the sequence '%252F' in the path section
of an incoming URL. This issue pointed out by Mark is in fact an unresolved,
but IMHO not in the scope of the mod_jk part but in the httpd parser.

I agree with Mark that the semantic of this literal should represent an encoded
<percent sign> followed by <digit 2><letter F>. And not an encoded slash.
But
the starting '%25' is decoded by the upstrem parser to an '%' and from that,
jk_canonenc() get a '%2F'. Without the patch, this would be encoded into
'%252F' again. With the patch, it would left as '%2F', and the semantics
changes. 

Said that, I think the upstream parser in addition SHOULD NOT decode a '%25' if
'AllowEncodedSlashes' is enabled. Because then  jk_canonenc  gets a '%252F' and
the patch might be extended to leave '%25' untouched, too.

Grüße

Guido

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message