Ignore bad percent encodings in paths during URIUtil.equalsIgnoreEncodings() #4033
Comments
|
Good catch. I'll submit a PR for this shortly. |
+ Allows for preserving decoded Strings like "X%YZ" Signed-off-by: Joakim Erdfelt <[email protected]>
|
Opened PR #4034 to address this with a lenient percent decoding solution. |
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
|
@wilkinsona we have a fix in place on PR #4034 that seems to address the issue you are encountering. In our codebase, that method only used in test cases and |
|
@joakime Thanks for taking a look at this so quickly. Sorry for the context that was lost in the overly minimal example of the problem. When the problem occurs in Jetty proper, The call to The |
|
Oh, wow, cool test. What does that file name (and parent path with |
In this particular case, the file isn't on disk. It's in |
…ignoreencodings Issue #4033 - Add Lenient percent decode in URIUtil
joakime
changed the title
|
@wilkinsona there's a 9.4.21-SNAPSHOT build available with these Lenient decode changes at the standard Jetty snapshot repository. https://oss.sonatype.org/content/repositories/jetty-snapshots/ Can you give it whirl on your tests? |
|
I've tried the 9.4.21 snapshot and the changes look good to me, @joakime. Thanks! |
|
Thanks for the feedback! |
|
@wilkinsona this fix is on the schedule for the Jetty 9.4.21 release. |
joakime
changed the title
|
I am worried that this fix has gone to far. Previously a request for |
gregw
added
High Priority
Sponsored
|
|
Added test to demonstrate bad percent encoded request Signed-off-by: Greg Wilkins <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
reverted decodePathBehaviour Signed-off-by: Greg Wilkins <[email protected]>
|
Opened PR #4272 |
updates after review Signed-off-by: Greg Wilkins <[email protected]>
* Modernizing testcase Signed-off-by: Joakim Erdfelt <[email protected]> * Issue #4033 Percent Encoded Bad Requests Added test to demonstrate bad percent encoded request Signed-off-by: Greg Wilkins <[email protected]> * Issue #4033 - adding sanity test for percent paths and checkAlias() Signed-off-by: Joakim Erdfelt <[email protected]> * Eliminating 9.3.0.RC0 dependency Signed-off-by: Joakim Erdfelt <[email protected]> * Issue #4033 - More tests for Resource checkAlias() behavior Signed-off-by: Joakim Erdfelt <[email protected]> * Issue #4033 - Splitting badDecodePath Signed-off-by: Joakim Erdfelt <[email protected]> * Issue #4033 - More badDecodePath tests Signed-off-by: Joakim Erdfelt <[email protected]> * Issue #4033 Percent Encoded Bad Requests reverted decodePathBehaviour Signed-off-by: Greg Wilkins <[email protected]> * testing pull request building * Issue #4033 updates after review Signed-off-by: Greg Wilkins <[email protected]>
This updates us to the newest Jetty verion. This required two changes. One was test changes around CVE-2016-2785 and the other involves our custom SslContextFactory. For background on CVE-2016-2785, see [TK-343](https://tickets.puppetlabs.com/browse/TK-343) about how certain percent encodings could be used to create a directory traversal attack. In [#4033](jetty/jetty.project#4033), the Jetty maintainers, in response to a user who got an NPE when attempting to serve files from his jar by percent encoding a jar path, loosened the percent encoding rules. Quickly after merging those loosened validation rules, it was reverted and the rules were additionally tightened up. As part of that tightening, some of the dangerous percent encodings Jetty used to let through (and we explicitly tested received an error from our middleware) no longer are allowed. Despite removing the assertion that we see the specific error message from our middleware we still check that the directory attacks receive a 404 and the primary handler never sees the request. Previously, Jetty deprecated the SslContextFactory that we extend to provide custom CRL reloading. It also moved the endpointIdentificationAlgorithm setting to specialized inner classes, either SslContextFactory.Server or SslContextFactory.Client depending on what defaults are required. With the latest updates attempting to extend the deprecated SslContextFactory raises and we must now extend the inner class SslContextFactory.Server. As a side effect we now no longer need to manually set the endpointIdentificationAlgorithm.
This updates us to the newest Jetty verion. This required two changes. One was test changes around CVE-2016-2785 and the other involves our custom SslContextFactory. For background on CVE-2016-2785, see [TK-343](https://tickets.puppetlabs.com/browse/TK-343) about how certain percent encodings could be used to create a directory traversal attack. In [#4033](jetty/jetty.project#4033), the Jetty maintainers, in response to a user who got an NPE when attempting to serve files from his jar by percent encoding a jar path, loosened the percent encoding rules. Quickly after merging those loosened validation rules, it was reverted and the rules were additionally tightened up. As part of that tightening, some of the dangerous percent encodings Jetty used to let through (and we explicitly tested received an error from our middleware) no longer are allowed. Despite removing the assertion that we see the specific error message from our middleware we still check that the directory attacks receive a 404 and the primary handler never sees the request. Previously, Jetty deprecated the SslContextFactory that we extend to provide custom CRL reloading. It also moved the endpointIdentificationAlgorithm setting to specialized inner classes, either SslContextFactory.Server or SslContextFactory.Client depending on what defaults are required. With the latest updates attempting to extend the deprecated SslContextFactory raises and we must now extend the inner class SslContextFactory.Server. As a side effect we now no longer need to manually set the endpointIdentificationAlgorithm.
|
There seems to have been a change in this area in Jetty 10 or 11. Before opening a new issue, I thought it best to mention it here to hopefully learn if it's intentional. A test that tries to access a static resource named |
|
What is the static filename (in the raw, without URI encoding/decoding) on your filesystem (be it a real filesystem, network filesystem, or jar/zip filesystem) Is it I use this tiny project to test encoding/decoding and static resources ... So far, I can access all of the content in the jar file. |
|
Thanks, @joakime. The file's in a jar file and it's named |
The problem is illustrated by the following test:
It fails with a
NumberFormatException:I discovered this while investigating this Spring Boot issue to see if Jetty was affected in any way as well. A more user-facing symptom of the problem in
URIUtilorTypeUtilis, I believe, that a static resource inMETA-INF/resourcesthat includes certain reserved characters in its name will be inaccessible.The text was updated successfully, but these errors were encountered: