On 25.10.2011 20:03, Christopher Schultz wrote:
> Rainer,
>
> On 10/23/2011 12:19 PM, [email protected] wrote:
>> +/* + * Find the first occurrence of path in uri tokenized by
>> "/". + * The comparison is done case insensitive. + */ +static
>> const char *find_path_in_uri(const char *uri, const char *path)
>> +{ + size_t len = strlen(path); + while (uri = strchr(uri,
>> '/')) {
>
> I think "//" in a URL will cause this loop to exit early, possibly
> avoiding this security check.
Why should it? strchr() goes to all occurances of '/' and the loop
will exit at the first occurance which is followed by path and '/' or
'0'. Note the not so nice strcmp() convention that it returns 0 for
equality, so !strncmp() means equality.
>> + uri++; + if (!strncmp(uri, path, len) &&
>
> strncmp doesn't use case-insensitive compare: will this ever match
> if you use "web-inf" (as below)?
Konstantin already observed that. I fixed it yesterday in r1188226.
>> + (*(uri + len) == '/' || + strlen(uri) ==
>> len)) { + return uri; + } + } + return
>> NULL; +} + static int uri_is_web_inf(const char *uri) { - if
>> (stristr(uri, "/web-inf")) { + if (find_path_in_uri(uri,
>> "web-inf")) { return JK_TRUE;
>
> This will return JK_TRUE if "web-inf" occurs at any place in the
> path, not just at the context level. Is that a problem? I can
> imagine that a request for /context/foo/WEB-INF/something might be
> valid.
But that was the original intention. Anything below web-inf should be
restricted from access, independent of how deeply below it is. That
was the original behaviour. Don't want to change that.
Thanks for the review!
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
