Martin, On 10/24/13 4:07 AM, Martin Grigorov wrote: > Hi, > > > On Wed, Oct 23, 2013 at 6:38 PM, Christopher Schultz < > ch...@christopherschultz.net> wrote: > >> All, >> >> I went looking into WebappClassLoader's validateJarFile() and filter() >> methods, and I noticed two things: >> >> 1. The error message for locating an illegal class being loaded from a >> JAR file references servlet spec 2.3 section 9.7.2. The current >> published version of the spec (3.0) is now section 10.7.2. Any >> objections to changing the message to reflect the new spec version >> supported by Tomcat and the new section number? >> >> > +1 > > >> 2. In spite of the above spec section, Tomcat only checks for >> javax.servlet.Servlet and/or javax.el.Expression and then rejects the >> entire JAR file. It doesn't look like classes such as java.lang.String >> or javax.xml.parsers.SAXParser, etc. are prohibited. This seems to be a >> spec violation. >> > > It is not allowed to create a custom class in java.* package. > You can probably put java.lang.String in a custom jar, but it will be the > same class as provided by the JDK. It could be a different version than the > runtime JRE though ... > So I agree that it should be rejected. > > I'm not 100% certain but I think it is OK to create custom classes in > javax.** package. So I think these should not be prohibited. > > >> >> 3. If a JAR file is found to contain a prohibited class, the entire JAR >> file is rejected. This behavior is not mandated by the servlet spec and >> may be over-reaching. >> > > Do you know what would be the performance effect of rejecting just the > problematic class and check the rest of the classes in the JAR ? > The current fail-fast behavior seems like a good optimization to me.
The current behavior is to loop-over the prohibited class list and call zipFile.getZipEntry() to check if any of those files exist in the ZIP file. I'm not sure exactly how getZipEntry works, but it may not exactly be the fastest way to scan for multiple (potential) ZIP entries. I have no performance data for any of this. I was thinking more about spec-compliance and protecting the web application against its own stupidity than anything else. >> I'm sure there are all kinds of practical reasons to only look for >> certain classes, and not to prohibit replacement of things like JSTL, >> etc. But it seems like this should be tightened-up a bit, even if >> through configuration things can be relaxed to their current state. >> >> Any thoughts? >> >> -chris >> >> >
signature.asc
Description: OpenPGP digital signature
