search

OWASP / owasp-java-encoder

The OWASP Java Encoder is a Java 1.5+ simple-to-use drop-in high-performance encoder class with no dependencies and little baggage. This project will help Java web developers defend against Cross Site Scripting!
https://owasp.org/www-project-java-encoder/
BSD 3-Clause "New" or "Revised" License
483 stars 112 forks source link

Transitive dependency on esapi #30

Closed fransonsr closed 4 years ago

fransonsr commented 5 years ago

The transitive dependency on esapi today picked up version 2.2.0.0-RC2 which was also released today. Unfortunately, there is a bug that broke our code in that version. With the "principle of least astonishment" I would not have expected to pick up a release-candidate build. Should not the dependency be limited to actual releases?

kwwall commented 5 years ago

@fransonsr I fear that we broke more than just the ESAPI GitHub Issue #488 that you referenced.I just pulled down your current code base and tried to compile it and got compilation errors:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.5.1:compile (default-compile) on project encoder-esapi: Compilation failure [ERROR] /home/kww/Code/GitHub/owasp-java-encoder/esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java:[128,13] org.owasp.encoder.esapi.ESAPIEncoder.Impl is not abstract and does not override abstract method getCanonicalizedURI(java.net.URI) in org.owasp.esapi.Encoder

And I suspect that there are a lot more subtle things as well. For instance, we previously compiled ESAPI with '-target 1.6', but starting with 2.2.0.0-RC1 (which was never pushed to Maven Central), we changed that to '-target 1.7'. But your README.md states that OWASP Java Encoder will work with JDK 1.5. Not if it is used in this mode it won't.

I'm not really sure what the whole idea / purpose of this "encoder-esapi" (aka, "ESAPI Thunk") is anyway. If it really is, as your esapi/pom.xml documentation states to "provides an easy way to plugin the Encoder Projects API into an implementation of ESAPI", I think it would be a lot more straightforward to simply tweak one's ESAPI.properties "ESAPI.Encoder" property with some appropriate OWASP Java Encoder class and work things that way. (I.e., take a path similar to what I did with getting ESAPI to work with the OWASP Java HTML Sanitizer project instead of using OWASP AntiSamy.) I think that what you have is likely to be way too brittle and @xeno6696 and I can't promise you that this will work without problems now or in the future. (For one thing, unlike Sun and Oracle, when we deprecate things, we eventually go back and actually remove them at some reasonable time!) However, if you want to work in the other direction as I described above, we probably can try to support that.

kwwall commented 5 years ago

And BTW, while we are on the topic of compatibility, Matt Seil fixed how ESAPI encoders were handling non-BMP characters in ESAPI GitHub Issue #300. So that fix is going to introduce a likely discrepancy between the reference ESAPI encoder and the OWASP Java Encoder Project. So I would highly encourage you to look at those fixes for the Java Encoder project.

fransonsr commented 5 years ago

@kwwall Just to clarify: I am merely an enthusiastic consumer, not an actual contributor. 😉

kwwall commented 5 years ago

Okay. Well maybe @manicode will eventually find time to respond then, once he gets done with all his presentations. :)

-kevin

Blog: http://off-the-wall-security.blogspot.com/ | Twitter: @KevinWWall NSA: All your crypto bit are belong to us.

On Tue, Apr 30, 2019, 02:11 Scott Franson notifications@github.com wrote:

@kwwall https://github.com/kwwall Just to clarify: I am merely an enthusiastic consumer, not an actual contributor. 😉

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/owasp-java-encoder/issues/30#issuecomment-487831161, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO6PGYOZ4KHK7HZAAS6K4LPS7PIVANCNFSM4HJIHXCQ .

jmanico commented 5 years ago

Hello Kevin and thanks for keeping ESAPI alive. What’s your question and how can I help?

-- Jim Manico @Manicode

On Apr 30, 2019, at 1:36 AM, Kevin W. Wall notifications@github.com wrote:

Okay. Well maybe @manicode will eventually find time to respond then, once he gets done with all his presentations. :)

-kevin

Blog: http://off-the-wall-security.blogspot.com/ | Twitter: @KevinWWall NSA: All your crypto bit are belong to us.

On Tue, Apr 30, 2019, 02:11 Scott Franson notifications@github.com wrote:

@kwwall https://github.com/kwwall Just to clarify: I am merely an enthusiastic consumer, not an actual contributor. 😉

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/owasp-java-encoder/issues/30#issuecomment-487831161, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO6PGYOZ4KHK7HZAAS6K4LPS7PIVANCNFSM4HJIHXCQ .

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

kwwall commented 5 years ago

@manicode @jmanico (sorry for the dups; not sure which of these you are using) -- Not a "question" so much as a "concern". Looks like ESAPI 2.2.0.0-RC2 release broke things more than just the way that this issue implies. See my earlier comments in this thread for the compilation errors it caused. If there is something that you need the ESAPI team to do as a result, then please create a GitHub issue for us. Thanks.

jmanico commented 5 years ago

I’m way out of position. Can you please submit a bug and test case for this in the Java Encoder Project? I don’t fully understand the problem but will look into it.

Aloha,

Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On Apr 30, 2019, at 7:27 AM, Jim Manico jim@manicode.com wrote:

Hello Kevin and thanks for keeping ESAPI alive. What’s your question and how can I help?

-- Jim Manico @Manicode

On Apr 30, 2019, at 1:36 AM, Kevin W. Wall notifications@github.com wrote:

Okay. Well maybe @manicode will eventually find time to respond then, once he gets done with all his presentations. :)

-kevin

Blog: http://off-the-wall-security.blogspot.com/ | Twitter: @KevinWWall NSA: All your crypto bit are belong to us.

On Tue, Apr 30, 2019, 02:11 Scott Franson notifications@github.com wrote:

@kwwall https://github.com/kwwall Just to clarify: I am merely an enthusiastic consumer, not an actual contributor. 😉

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/owasp-java-encoder/issues/30#issuecomment-487831161, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO6PGYOZ4KHK7HZAAS6K4LPS7PIVANCNFSM4HJIHXCQ .

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

jmanico commented 5 years ago

Ah I see it’s a bug already... On it

-- Jim Manico @Manicode Secure Coding Education +1 (808) 652-3805

On Apr 30, 2019, at 10:34 AM, Kevin W. Wall notifications@github.com wrote:

@manicode @jmanico (sorry for the dups; not sure which of these you are using) -- Not a "question" so much as a "concern". Looks like ESAPI 2.2.0.0-RC2 release broke things more than just the way that this issue implies. See my earlier comments in this thread for the compilation errors it caused. If there is something that you need the ESAPI team to do as a result, then please create a GitHub issue for us. Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jmanico commented 4 years ago

If this is still an issue please re-open.