New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Canonicalizing "%Device% changes the meaning of the input string #306
Comments
@kwwall this is an invalid use-case. The only thing you should do with a canonicalized string is to use it for further validation. Application implementations should default to storing data as-is, and follow the strategy of output-escaping all user input. For the input in question, the part Missing in this question also, is that the relevant encoding properties in The canonicalize function is doing exactly what it was intended to do. |
Here is the relevant JUnit test to reproduce:
|
Because %de is a legit URL encoded character? --Jeff From: Matt Seil <notifications@github.commailto:notifications@github.com> Here is the relevant JUnit test to reproduce:
Reply to this email directly or view it on GitHubhttps://github.com//issues/306#issuecomment-176420289. |
@planetlevel Like @xeno6696 said, apparently it is a represents the 'Þ' character, whatever the heck that is. The only way that I see around this is to create a custom Codec that excludes certain things and use that rather than PercentCodec (which presumably is what is being used here) and add that to an explicit list of Codecs passed to the DefaultEncoder similar to what is sketched out in http://static.javadoc.io/org.owasp.esapi/esapi/2.1.0/org/owasp/esapi/Encoder.html#canonicalize%28java.lang.String,%20boolean,%20boolean%29. However, I agree with Matt's assessment that what is being proposed as a bug here is just in invalid use case and I don't think that we should try to support it. My vote is to close this issue. Does anyone object? |
I think Jeff is assuming UTF-8 URL encoding. I've worked on systems using ISO 8859-1 and %DE would be a legal encoding there. |
No objections to closing. |
I'm reaching a bit out of my depth, so please forgive me if I'm I think there may still be a bug here. I've written a unit test which org.junit.ComparisonFailure: expected:<[%De]> but was:<[Þ]> I think this means the immunity list for the PercentCodec has a bug? Test class attached. On 1/29/2016 5:34 AM, Matt Seil wrote:
|
It appears that your test is asserting that the encoded string should be the same as the unencoded string? This is not correct in the case of the input |
I spoke with @jeremiahjstacey in person, and the problem is a legitimate bug. If you crack open the CodecTest.java unit test file, the immunity part of the API was only ever tested with an This is a good catch. Congratulations! I will file an appropriate bug and attach your unit test. |
From shilpi.a...@gmail.com on May 23, 2013 04:45:26
What steps will reproduce the problem? 1. Take String "%Device%.
2. Canonicalize it using Canonicalize method
3. Now do EncodeForHTML or simple display the result string from Canonicalize. What is the expected output? What do you see instead? The output has needs to be encoded for html, should display as "%Device%" in browser,Instead we see "Þvice%" What version of the product are you using? On what operating system? 2.0rc Does this issue affect only a specified browser or set of browsers? all Please provide any additional information below. We are using these APIs heavily. Please provide an estimate fix date.
Original issue: http://code.google.com/p/owasp-esapi-java/issues/detail?id=300
The text was updated successfully, but these errors were encountered: