search

ESAPI / esapi-java-legacy

ESAPI (The OWASP Enterprise Security API) is a free, open source, web application security control library that makes it easier for programmers to write lower-risk applications.
https://owasp.org/www-project-enterprise-security-api/
Other
603 stars 364 forks source link

remove dependency on servlet API (use case where not needed) #776

Closed JosephWitthuhnTR closed 1 year ago

JosephWitthuhnTR commented 1 year ago

I have recreated PR #772, except this time I have properly signed the commit.

This change deals with issue #767.

It does not attempt to resolve the overall issue with Servlet API version compatibility. What it does is remove the dependency on Servlet API for the use case where a user is trying to use the logger functionality, but has "logServerIP" set to false. When not logging the server IP, there is no reason for this code to depend on Servlet API at all.

This allows us to avoid runtime exceptions if we use this code with the new Jakarta package version of Servlet API, in cases where logServerIP is disabled.

Again, this is not a fix for the overall problem. This is a small change that will let some limited use cases not fail due to the problem (so that those use cases can get by until this problem is solved in a more holistic way).

The change stems from part of this discussion: https://github.com/ESAPI/esapi-java-legacy/discussions/768#discussioncomment-4672270

JosephWitthuhnTR commented 1 year ago

@kwwall / @jeremiahjstacey I have recreated PR #772 , except this time I have properly signed the commit.

kwwall commented 1 year ago

@JosephWitthuhnTR - Looks like this is failing for some reason on one of the surefire plugins. I don't have time to investigate it at lunch time. Will try to look at it later today.

JosephWitthuhnTR commented 1 year ago

@kwwall The failure looks related to my change. I'll investigate:

ServerInfoSupplierTest.verifyOutputNullAppName:84 expected:<[LOCAL_ADDR:99999]/null/verifyOutputNu...> but was:<[]/null/verifyOutputNu...>

JosephWitthuhnTR commented 1 year ago

@kwwall I had it right on my original PR, but somehow messed it up when I recreated this PR with the signed commit. Either way, I've now fixed the unit test on this PR.

kwwall commented 1 year ago

@kwwall I had it right on my original PR, but somehow messed it up when I recreated this PR with the signed commit. Either way, I've now fixed the unit test on this PR.

Dang. I should have probably looked closer, but I assumed you just copy/pasted from the old one or used the same old files, so my bad. Guess that;s why we have these GitHub actions as a sanity check.

ebarry-vp commented 1 year ago

Any idea on when this fix will be released soon? Thanks!

kwwall commented 1 year ago

The bottleneck for this before doing this next new release is that I need to write up:

  1. Release note (generally takes 3+ hours)
  2. Write up a security bulletin in regards to the CVE-2023-24998 vulnerability from the commons-fileupload-1.4.jar dependency. (Probably also at least 3 hrs or more, depending on how long it takes me to analyze their commons-fileupload code to see if the CVE is exploitable through the way that ESAPI uses it.

Thus far, I have not be able to find enough free time to do either of those things and I'm not going to do a release until I can produce those artifacts as part of the new release. And given that tax time is upon us, that's something else that I need to attend to so free time is scarce right now.

P.S.- In reality, I think you should be able to work around the problem you were having by just changing the ESAPI logger to use SLF4J rather than using the default JUL for logging.

ebarry-vp commented 1 year ago

P.S.- In reality, I think you should be able to work around the problem you were having by just changing the ESAPI logger to use SLF4J rather than using the default JUL for logging.

@kwwall - Please can you help us how to do this work around to avoid this error: java.lang.NoClassDefFoundError: javax/servlet/ServletResponse

I tried using this: ESAPI.Logger=org.owasp.esapi.logging.slf4j.Slf4JLogFactory That still didn't prevent the code from running into this issue.

We really need the jakarta changes and folks that need javax, can still point to older versions in my opinion. Any thoughts?

Thanks!

kwwall commented 1 year ago

@ebarry-vp and others - Disregard that last remark about using SLF4J. My memory was not serving me well and I mistakenly thought that the class that was the PR was for was one belonging to the org.owasp.esapi.logging.java package which would have limited the effect to JUL, but it was not. The PR was for the class org.owasp.esapi.logger.appender.ServerInfoSupplier.

However, the Maven Shade approach that @planetlevel mentioned here or the Jakarta EE transformer that @arjantijms mentioned here for Eclipse ought to theoretically work. If those approaches don't work, then a lot of developers are going to be totally screwed.