It got further, but ran into another security check within the org.apache.catalina.core.ApplicationContext. I'm using the 10.1.35-SNAPSHOT version as that is closest to my current version, and easiest to test as a result. This is the block of code having trouble now (linking into 10.1.34): https://github.com/apache/tomcat/blob/10.1.34/java/org/apache/catalina/core/ApplicationContext.java#L386-L396
This code: 1. takes the URL (e.g. /customers/customer/A%2FB%5CC) 2. decodes it (e.g. /customers/customer/A/B\C) 3. normalizes the decoded url with the replaceBackSlash parameter hardcoded to true, thus forcing the decoded '\' character to become a '/' character (e.g. /customers/customer/A/B/C) 4. then it compares the decoded URL to the normalized URL, and if they are not equal, an IllegalArgumentException is created and a 500 response is returned. It seems that this area of the code would need to be aware of the encodedReverseSolidusHandling configuration as well. Thank you, James On Tue, Jan 21, 2025 at 1:20 PM Mark Thomas <ma...@apache.org> wrote: > On 21/01/2025 14:15, James Matlik wrote: > > Hello Mark, > > > > Yes, I would be available to test a snapshot if this gets implemented. > > Latest (as I type this) Tomcat 12 build with the new feature: > > https://repository.apache.org/content/groups/snapshots/org/apache/tomcat/tomcat/12.0.0-M1-SNAPSHOT/tomcat-12.0.0-M1-20250121.171115-315.tar.gz > > The snapshots for the 11.0.x, 10.1.x and 9.0.x are still building. > Generally, you'll find them under: > > > https://repository.apache.org/content/groups/snapshots/org/apache/tomcat/tomcat/ > > Anything built after 1700 UTC today should have the fix. Make sure you > look for the snapshot for the current dev version for each release branch. > > The Connector attribute is called encodedReverseSolidusHandling > > Let the list know how you get on. > > Mark > > > > > > -James > > > > On Tue, Jan 21, 2025 at 8:17 AM Mark Thomas <ma...@apache.org> wrote: > > > >> On 18/01/2025 16:18, James Matlik wrote: > >>> I agree with everything you have said. As the config options stand > today, > >>> the allowBackslash seems to implement part of encodeSolidusHandling. > >>> > >>> While encodeSolidusHandling supports: > >>> * REJECT - Return 400 on encoded / > >>> * DECODE - Decodes the / > >>> * PASS_THROUGH - Passes the encoded / as is > >>> > >>> The allowBackslash supports two mostly analogous settings: > >>> * false - REJECT > >>> * true - DECODE, but it Decodes as / instead of \ > >> > >> Not quite. Decoding is separate. > >> > >> Currently, %5C always gets decoded to \. allowBackslash then determines > >> whether any \ (originally encoded or not) characters in the URL will be > >> permitted and converted to / or the URL rejected. > >> > >> The decoding and conversion are partially independent. For example, > >> while it may be required that an encoded sequence %5C passed through to > >> the application, whether an unencoded \ is rejected or converted to / is > >> an independent choice. > >> > >> From a security point of view, it is necessary to always convert \ to > / > >> - at least on Windows anyway - else all sorts of nasty security things > >> could happen such as bypassing security constraints. > >> > >> We should be able to fix this for the next round of releases (early > >> Feb). I am currently looking at whether a new option to effectively > >> replace allowBackslash is sufficient or whether an new option is needed > >> in addition to allowBackslash. > >> > >> My current thinking is that we should add encodeReverseSolidusHandling > >> with the same options as encodeSolidusHandling and leave the > >> convert/reject decision for unencoded \ to allowBackslash. > >> > >>> IMO, an ideal implementation would introduce > encodeReverseSolidusHandling > >>> to behave the same as encodeSolidusHandling, but maybe add a > >>> DECODE_AS_SOLIDUS. Then the allowBackslash setter would set the > >>> encodeReverseSolidusHandling variable to DECODE_AS_SOLIDUS or REJECT > >> (true > >>> vs false) for backward compatibility, but should probably be > deprecated. > >>> The code that normalizes the URL can remove any special handling of > the \ > >>> character because all enforcement can be applied within UDecoder as is > >> done > >>> with the / character today. > >> > >> I don't think DECODE_AS_SOLIDUS is necessary although it could always be > >> added as an additional option later if necessary. > >> > >>> This would make the handling of \ more consistent with /, and I imagine > >> the > >>> security concerns between the two remain about equal. Though, like you > >>> said, others on the team have already thought deeply about this, so I > may > >>> be missing something important. > >>> > >>> If I were to pursue getting a change like this made to Tomcat, where > >> should > >>> I start? > >> > >> This is the way to start the process. A discussion on the users list > >> that clearly explains the problem and what a solution might look like. > >> > >>> I know this would be a slow process. > >> > >> There is a good reason for the feature, the implementation looks to be > >> backwards compatible and not too invasive, there isn't simple extension > >> point you can use to implement and one or more committers are interested > >> in solving the problem then there is a good chance it will be in the > >> next round of releases (early Feb). > >> > >>> Or, if I'm better off working around the core functionality, would you > >> have > >>> any suggestions on how? I see the UDecoder recently changed to support > >>> encoded % characters. I considered using a double encoded \ hack to > >>> effectively pass through, but that won't work now. Plus, I wasn't able > to > >>> figure out how to add custom logic before the URL decode and normalize > to > >>> add my work around encoding on the server side, as doing so in the > client > >>> side isn't feasible. > >>> > >>> Ideally, I wouldn't need to maintain a custom build of Tomcat > >> indefinitely. > >> > >> There isn't an easy (or any) extension point to implement this. It would > >> have to be a custom Tomcat build. > >> > >> Are you able to test some snapshot builds if this gets implemented? > >> > >> Mark > >> > >>> > >>> Thanks, James > >>> > >>> On Fri, Jan 17, 2025, 10:00 AM Christopher Schultz < > >>> ch...@christopherschultz.net> wrote: > >>> > >>>> James, > >>>> > >>>> On 1/17/25 8:04 AM, James Matlik wrote: > >>>>> When I'm talking about path parameters, it is in the context of how > >> Open > >>>>> API/Swagger defined them: > >>>>> https://swagger.io/docs/specification/v3_0/describing-parameters/ > >>>> > >>>> Okay, that helps clear things up. In the URL specification (inherited > by > >>>> HTTP) defines them differently: > >>>> > >>>> https://www.rfc-editor.org/rfc/rfc3986.html#section-3.3 > >>>> > >>>> The term "path parameter" you are using is actually a "path segment" > in > >>>> URL/HTTP terms. > >>>> > >>>> The problem you are running into is that encoded slashes are often > used > >>>> to attack servers by performing path-traversals or avoiding security > >>>> constraints. Let's say you are on Windows and there is a security > >>>> constraint that says "only admin users can access files in > >>>> D:\docroot\(ON\QC)". You have mapped URL-/ to physical-D:\docroot. > >>>> (Admittedly this is very contrived). > >>>> > >>>> The attacker says "well, %29 is a backslash, so lemmie just check to > see > >>>> if I can slip one past the security system" and requests > >>>> http://localhost:8080/group/%28ON%5CQC%29%20LOCAL. If the server is > >>>> fooled, the attacker gets to request that resource in that directory. > >>>> > >>>> My guess is that Tomcat's "URL cleaner" is seeing that \, attempting > to > >>>> "sanitize" it, and converting it to a / because that's the local > >>>> filesystem path separator. This makes security constraints much more > >>>> difficult to bypass. > >>>> > >>>> My naïve reading of your desire to have > >>>> encodeSolidusHandling=PASS_THROUGH behave the same for / and \ seems > >>>> reasonable if a misnomer. I say "naïve" because others on this team > have > >>>> spent countless hours of their lives mulling these things over and > >>>> getting the logic "just right". > >>>> > >>>> I say it's a misnomer because "solidus" means literally / and not \. > >>>> encodeReverseSolidusHandling? encodeAntiSolidusHandling? > >>>> > >>>> The encodeSolidusHandling configuration setting is there so we can > have > >>>> a very secure default-configuration that prevents attacks on naive > >>>> applications. In your case, you have an application that (presumably) > >>>> knows exactly what it is doing, and therefore you have the option to > >>>> pass-through those solidus...es. Solidii? Whatever. > >>>> > >>>> I think that actually having a separate configuration setting for > >>>> antisolidii handling makes more sense than having that existing > setting > >>>> perform double-duty, since each one of those you allow to pass-through > >>>> opens the door slightly wider to an attacker. > >>>> > >>>> -chris > >>>> > >>>> > >>>> --------------------------------------------------------------------- > >>>> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org > >>>> For additional commands, e-mail: users-h...@tomcat.apache.org > >>>> > >>>> > >>> > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org > >> For additional commands, e-mail: users-h...@tomcat.apache.org > >> > >> > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org > For additional commands, e-mail: users-h...@tomcat.apache.org > >
