On 25.10.2011 20:03, Christopher Schultz wrote: > Rainer, > > On 10/23/2011 12:19 PM, rj...@apache.org 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: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org