Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use of vulnerable commons-httpclient:commons-httpclient:3.1 dependency #372

Closed
pachulo opened this issue May 10, 2016 · 58 comments
Closed

Use of vulnerable commons-httpclient:commons-httpclient:3.1 dependency #372

pachulo opened this issue May 10, 2016 · 58 comments

Comments

@pachulo
Copy link

pachulo commented May 10, 2016

Hi! Analyzing a project where ESAPI is used I found out that has antisamy as dependency, and that this project has commons-httpclient:3.1 as a dependency:

org.owasp.esapi:esapi:jar:2.1.0.1:compile

....

+- org.owasp.antisamy:antisamy:jar:1.5.3:compile
|  +- net.sourceforge.nekohtml:nekohtml:jar:1.9.16:compile
|  \- commons-httpclient:commons-httpclient:jar:3.1:compile

...

This version is in EOL state:

And the new one (4.x) breaks the API:

The OWASP Dependency Check tool tells us that the old version could be affected by 3 CVEs:

And while I'm not really sure, I cannot really recommend to anybody the use of the ESAPI library in this state. I'm I missing something?

Thank you!
@xeno6696
Copy link
Collaborator

I think this will only affect users who implement the WAF functionality that ESAPI provides. I would have to check the code to be sure, but should we publish an advisory, @kwwall?

@planetlevel
Copy link

Check to see if library is even used and whether that use allow vuln to be exploited. If people did a release for every out of date dependency, we would fill up CVE.

--Jeff

On Tue, May 10, 2016 at 7:44 AM -0700, "Matt Seil" <notifications@gh.neting.ccmailto:notifications@github.com> wrote:

I think this will only affect users who implement the WAF functionality that ESAPI provides. I would have to check the code to be sure, but should we publish an advisory, @kwwallhttps://github.com/kwwall?

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHubhttps://github.com//issues/372#issuecomment-218179637

@pachulo
Copy link
Author

pachulo commented May 10, 2016

OK, I think I found out that the only use of HttpClient in org.owasp.antisamy is in this class:

So at least we know were it could be vulnerable or even patched in org.owasp.antisamy.

What do you mean with WAF functionality @xeno6696 ? In ESAPI I found the use of antisamy here:

In the method "invokeAntiSamy", but I'm not really sure how it works or how the antisamy policy applies:

@kwwall
Copy link
Contributor

kwwall commented May 10, 2016

Agree with @planetlevel; I do secure code inspections in my day job and there are two things that we do not cite. One is CVEs are are indirect dependencies that some other library requires (e.g., in this case, antisamy is using Apache Commons HttpClient, but ESAPI does not use it directly), and the other is where the CVE is not exploitable in the context that we are using it.

@xeno6696 As far as ESAPI goes, the use of antisamy goes beyond the use of the WAF functionality. There are also some methods where it is used in the Validator feature as well. However, I am not sure what you mean by "should we (presumably meaning ESAPI) publish an advisory?". Do you mean should we fill for a CVE by Mitre? I don't really think they would assign one to us. For one thing, technically if anything is vulnerable, it would be AntiSamy and AFAIK, we are using the latest release of that. So the responsibility would need to be on the AntiSamy team to see if any of these CVEs originating from Apache Commons HttpClient 3.1 are exploitable in the context of how they are used within AntiSamy, and if so, I would expect that AntiSamy (w|sh)ould issue a new release with a version of HttpClient that is not vulnerable. If we (ESAPI) were to make that change, there is a potential (probably unlikely) that we could break how AntiSamy functions within ESAPI. [Note: our JUnit tests in that area are minimal and I would not expect those existing tests to catch that. We would almost have to--minimally--run the AntiSamy JUnit tests.]

So I assume that @xeno6696 was not referring for ESAPI filing a CVE with Mitre for such an "advisory". But if not that, what then? And a better question is, what if it were exploitable in an ESAPI context? What should we be doing in that case, besides asking the AntiSamy team for a fix?

@pachulo As far as your individual use, if you are concerned, you certainly could try ESAPI with an updated version of HttpClient (which I think is now part of HttpComponents, right?) and see what breaks. Since they have changed the major number (went from '3' to '4'), I would not be the least surprised if things would break though.

I think the long term approach to this is that we need to look elsewhere for this type of functionality. I already had started working on a branch to use Mike Samuel's OWASP Java HTML Sanitizer. (I think that branch might have been migrated to GitHub as well.) I was working on it primarily because 1) it had zero dependencies and AntiSamy had many and this allowed me to reduce the total # of overall dependencies for ESAPI, 2) it was faster, 3) if is better supported. Unfortunately, it was one of those things that got lost in the move from Google Code to GitHub, but if anyone wants to take it up, I'd be willing to help out. IIRC, there were not any major changes required. Let me know if anyone is interested in working on that.

@kwwall
Copy link
Contributor

kwwall commented May 10, 2016

@pachulo Yes, AntiSamy is used with the Validator methods in ESAPI. I noted that in my previous post. Did you confirm that any of these CVEs are actually exploitable by AntiSamy in the way that it uses them? I will try to look at these CVEs later and respond tonight or tomorrow.

@pachulo
Copy link
Author

pachulo commented May 10, 2016

@kwwall no, not really, I was just trying to find out why it was being imported and where it was being used.

Is this the branch you're talking about?

@kwwall
Copy link
Contributor

kwwall commented May 10, 2016

Yep, that would be the branch. Bo sure if everything was actually checked
in at the time it was migrated though bc I wasn't given sufficient advance
warning. But if not, I can probably dig up the other missing pieces.
They're on my laptop hard drive somewhere.

-kevin
Sent from my Droid; please excuse typos.
On May 10, 2016 12:17 PM, "pachulo" notifications@github.com wrote:

@kwwall https://github.com/kwwall no, not really, I was just trying to
find out why it was being imported and where it was being used.

Is this the branch you're talking about?

https://github.com/ESAPI/esapi-java-legacy/tree/kww-java-html-sanitizer

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#372 (comment)

@kwwall
Copy link
Contributor

kwwall commented May 10, 2016

s/Bo sure/But not sure/

Geesh; I need another cup of coffee.

-kevin
Sent from my Droid; please excuse typos.
On May 10, 2016 12:29 PM, "Kevin W. Wall" kevin.w.wall@gmail.com wrote:

Yep, that would be the branch. Bo sure if everything was actually checked
in at the time it was migrated though bc I wasn't given sufficient advance
warning. But if not, I can probably dig up the other missing pieces.
They're on my laptop hard drive somewhere.

-kevin
Sent from my Droid; please excuse typos.
On May 10, 2016 12:17 PM, "pachulo" notifications@github.com wrote:

@kwwall https://github.com/kwwall no, not really, I was just trying to
find out why it was being imported and where it was being used.

Is this the branch you're talking about?

https://github.com/ESAPI/esapi-java-legacy/tree/kww-java-html-sanitizer

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#372 (comment)

@xeno6696
Copy link
Collaborator

Actually, if you trace the original dependency tree provided by @pachulo,

+- org.owasp.antisamy:antisamy:jar:1.5.3:compile
|  +- net.sourceforge.nekohtml:nekohtml:jar:1.9.16:compile
|  \- commons-httpclient:commons-httpclient:jar:3.1:compile

The actual project at fault is NekoHtml, which is a dependency of AntiSamy, and in turn esapi.

I can update antisamy appropriately, but it doesn't look like that OWASP ever migrated the project to github. @kwwall is that something you would have to do or could I add it under the ESAPI umbrella?

@xeno6696
Copy link
Collaborator

I tested compiling antisamy with the most recent NekoHTML version, 1.9.22, and it still has the dependency on commons httpclient.

@xeno6696
Copy link
Collaborator

I created an issue with the NekoHTML team.

@mguillem
Copy link

NekoHTML has no dependency on HttpClient, neither in 1.9.16 nor in latest release.

@xeno6696
Copy link
Collaborator

This is verified, I checked out the source, and maven reports no dependencies. Probably should have done that first. /smh

@xeno6696
Copy link
Collaborator

xeno6696 commented May 23, 2016 •
edited
Loading

@kwwall, when trying to find new antisamy code, I discovered a github project that (claims) to have already converted Antisamy to httpclient 4: https://github.com/marcust/antisamy-httpclient4

Should I develop my own conversion or should we contact this guy?

@planetlevel
Copy link

By all means contact. But I can convert if need be. Have some experience with HttpClient.

--Jeff

From: Matt Seil <notifications@gh.neting.ccmailto:notifications@github.com>
Sent: Monday, May 23, 2016 5:29 PM
Subject: Re: [ESAPI/esapi-java-legacy] Use of vulnerable commons-httpclient:commons-httpclient:3.1 dependency (#372)
To: ESAPI/esapi-java-legacy <esapi-java-legacy@noreply.gh.neting.ccmailto:esapi-java-legacy@noreply.github.com>
Cc: Mention <mention@noreply.gh.neting.ccmailto:mention@noreply.github.com>, Jeff Williams <jeff.williams@contrastsecurity.commailto:jeff.williams@contrastsecurity.com>

@kwwallhttps://github.com/kwwall, when trying to find new antisamy code, I discovered a github project that has (claims) to have already converted Antisamy to httpclient 4: https://github.com/marcust/antisamy-httpclient4

Should I develop my own conversion or should we contact this guy?

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com//issues/372#issuecomment-221101919

@xeno6696 xeno6696 mentioned this issue May 24, 2016
Open
@augustd
Copy link
Contributor

augustd commented May 3, 2017

While it is true that many CVEs in 3rd party code are not technically exploitable in the applications that use them, they cause no end of trouble for corporate security departments that must track and validate them nonetheless. We, as purveyors of a security library, should be particularly sensitive to that fact. We should strive to not include vulnerable libs that will trip alarms and create extra work for the people who should be our biggest advocates.

@augustd
Copy link
Contributor

augustd commented May 3, 2017

See my comment on the other thread: marcust/antisamy-httpclient4#1 (comment) I believe that version can be a direct replacement for the existing AntiSamy (and it also has accessible source code, in GitHub, etc). I built it yesterday and all original AntiSamy tests pass.

@kwwall
Copy link
Contributor

kwwall commented May 5, 2017

@augustd The question is, have you uploaded that version referenced in the previous comment to Maven Central? If so, what is it's version #? ESAPI is currently using Antisamy 1.5.5. ESAPI needs a version of Antisamy that uses some version of Commons Httpclient that is not vulnerable to CVE-2015-5262, CVE-2014-3577, and CVE-2012-6153. (Yes, I know that none of these 3 CVEs are exploitable in the context of how Antisamy uses them, but unfortunately, OWASP Dependency Check does not know that.) The latest version of commons-httpclient-4.5.1 should work, but some earlier version may work as well. (I really haven't chased down these 3 CVEs to see the earliest version of Apache Commons Httpclient that will work.) I suppose one option for ESAPI is to suppress the warnings for these CVEs, but I really hate to do something like that for transitive ESAPI dependencies.

@augustd
Copy link
Contributor

augustd commented May 9, 2017

I believe I can release it to maven central -at least I am able to do that for OWASP Security Logging. But I feel like we first we need a repo where the "official" source can live. @marcust has indicated that he is not interested in developing it further.

I'm happy to fork it into a new repo under /augustd and give access to anyone that needs it, but maybe it would be better off under github.com/OWASP. Who has the ability to create new projects there?

@xeno6696
Copy link
Collaborator

@kwwall this sounds like an easy issue to fix, and I agree, the AntiSamy code needs to live under the OWASP banner.

@kwwall
Copy link
Contributor

kwwall commented Jul 26, 2017

@xeno6696 @augustd Wait, are we talking about AntiSamy (which was the original point, as I believe that particular issue should be fixed now; run the Maven dependency-check goal to verify) or are we discussing OWASP Security Logging here?

Not that it really matters that much... there are quite a few projects including OWASP Flagship projects of ZAP and Dependency Check that do not live under the github.com/OWASP banner. Also Arshan has already put up the latest AntiSamy versions (1.5.6, I think) on Maven Central and he is hosting that on his GitHub repo under github.com/nahsra/antisamy. Likewise ESAPI itself is not hosted under the OWASP banner per se although it clearly is an OWASP project.

So, to @augustd I would say just host this OWASP Security Logging under your own site. Permissions will be a lot easier for you to control than if you want to host it on github.com/OWASP and by hosting it there that doesn't imply any sort of support or anything else for it anyhow. So other than the name recognition, I'm not sure of the benefits. If you make the links to it from the OWASP wiki pages people will still find it. (If you still want to put it up on the OWASP GitHub site, I would suggest sending an email to the OWASP Leaders List and see if someone there knows who to contact about it.)

@augustd
Copy link
Contributor

augustd commented Jul 26, 2017

The vulnerable version of commons-httpclient is imported through AntiSamy, so it would need to be fixed there. I checked the latest version of AntiSamy that is available in Maven Central (1.5.5) and it still uses commons-httpclient 3.1:

[INFO] +- org.owasp.antisamy:antisamy:jar:1.5.5:compile
[INFO] |  +- net.sourceforge.nekohtml:nekohtml:jar:1.9.22:compile
[INFO] |  \- commons-httpclient:commons-httpclient:jar:3.1:compile
[INFO] |     \- commons-codec:commons-codec:jar:1.2:compile

The ideal course of action would be to get @nahsra to accept a pull request from https://github.com/marcust/antisamy-httpclient4, publish a new version (AntiSamy 2.0?) to Maven Central, and then update ESAPI to use that.

@xeno6696
Copy link
Collaborator

Wait... this is github. Why wait for a pull request? Just fork the dang thing and issue the pull request yourself!

@xeno6696
Copy link
Collaborator

There. I cloned it to here: https://github.com/ESAPI/antisamy

@xeno6696
Copy link
Collaborator

Cloned the other:

https://github.com/xeno6696/antisamy-httpclient4

@xeno6696
Copy link
Collaborator

Here be the diff.

I just pulled the repos together and am resolving unit test differences.

This shall be wrapped up shortly.

repoDiff.zip

@kwwall
Copy link
Contributor

kwwall commented Jul 27, 2017 via email

@xeno6696
Copy link
Collaborator

My plan was to clone both repos to my space, and then generate the final PR from my space to ESAPI's.

Belongs here with us anyways.

@kwwall
Copy link
Contributor

kwwall commented Jul 27, 2017 via email

@xeno6696
Copy link
Collaborator

Fixed the unit test in question. Have a Fork of antisamy, and its ready to be released, just need to know next steps. How long should we wait for a reply?

@kwwall
Copy link
Contributor

kwwall commented Jul 27, 2017 via email

@xeno6696
Copy link
Collaborator

Created PR 11 and deleted ESAPI/antisamy
|
nahsra/antisamy#11

@nahsra
Copy link

nahsra commented Jul 27, 2017 via email

@nahsra
Copy link

nahsra commented Jul 27, 2017 via email

@kwwall
Copy link
Contributor

kwwall commented Jul 27, 2017 via email

@augustd
Copy link
Contributor

augustd commented Jul 27, 2017

Submitted PR #407 to update to the latest AntiSamy (and XOM) dependency.

@adetlefsen-rms
Copy link

Can we get a new ESAPI version released to Maven now? This will clear a few Jiras for me...

@xeno6696
Copy link
Collaborator

@kwwall, care to make an announcement for "last call for issues" on the main list? I'd say we got a lot done for the release so far.

@kwwall
Copy link
Contributor

kwwall commented Jul 28, 2017

@xeno6696 Yet, I can write up something over the weekend, although I'm not really sure what a "last call for issues" means. I don't think GH gives you a way to 'star' or 'watch' individual issues or vote them up in priority, and a "last call for issues" certainly is not going to get people to sign up (at least in mass droves) for a final bug fix push and submitting PRs. It may get people to submit some new bugs that they've just been holding off on, but it's not clear that's a win for us either. At worst, we'd have to hold off on the release to fix some critical bug with security implications, and at best we'll just get more issues that we won't likely to fix before the release and find it disheartening. But whatever.

As far as setting dates for a commit freeze, let me see how far I get over the weekend in terms of addressing some small crypto issues. Unfortunately so much time has gone by, I have unintentionally caused my own merge conflicts!

@kwwall
Copy link
Contributor

kwwall commented Jul 28, 2017

@xeno6696 Oh, one more thing...since this AntiSamy / HttpClient vulnerable dependency has been cleared up, can we delete ESAPI/AntiSamy before someone forks it because they think we are maintaining it now rather than @nahsra ? Thanks.

@xeno6696
Copy link
Collaborator

xeno6696 commented Jul 29, 2017 via email

@augustd
Copy link
Contributor

augustd commented Aug 7, 2017

Can we get a new minor version released with the fix to this bug?

@kwwall
Copy link
Contributor

kwwall commented Aug 8, 2017

@augustd No promises on that. While it would make sense all around the web (the ESAPI security bulletin, various emails to the ESAPI-Users and ESAPI-Dev mailing lists, several security conferences where I've spoken, etc.) I promised that ESAPI 2.1.1 would be the version that finally addresses / fixes CVE-2013-5960. That's why the last release was 2.1.0.1 instead of 2.1.1.0 and why this one will likely be 2.1.0.2 rather than 2.1.1.0. If we had more hands to assist in writing up release notes, documentation, etc. maybe, but I've been spending about 80% of my time on stuff not directly related to CVE-2013-5960. and about 5% of my time fighting with Git and Maven.

While I absolutely agree that it probably makes sense to bump to 2.2.0.0 (if for no other reason than dropping support for JDK 6), I'm not sure I want to deal with the fall-out of why isn't the CVE fixed if we are not at 2.2.0.0. Yes, I painted myself into a box and one based on Git and Maven issues, I am finding difficult to crawl out of. Without @xeno6696 help, there might not have even been a 2.1.0.1 release. (The major impetus for that release was a Java deserialization issue that SourceClear found and reported to me and I wanted to get it fixed before I had to address it via the red tape of a CVE, which is a major PITA and I swear never again. Sigh.)

@ruebot ruebot mentioned this issue Aug 22, 2017
Closed
@rlevine1106
Copy link

rlevine1106 commented Nov 6, 2017 •
edited
Loading

@kvwall Sounds like there is a version that fixes the http-commons issue, however the latest version I see in maven central is 2.1.0.1. Any updates ?

@xeno6696
Copy link
Collaborator

xeno6696 commented Nov 6, 2017

There is a large number of bugfixes completed that are ready for a new release, but there is one major bug that @kwwall has been working on a private repo that is holding up the upcoming release.

@rlevine1106
Copy link

rlevine1106 commented Nov 6, 2017 •
edited
Loading

@xeno6696 From the thread, it sounds like the latest antisamy, resolves the commons-httpclient CVE finding. Is there any issue with overriding antisamy to the latest version in my project while waiting on the new release. That way esapi 2.1.0.1 doesn't pull in the transitive dependency of commons-httpclient

@augustd
Copy link
Contributor

augustd commented Nov 6, 2017

Can we get a version 2.1.0.2 in the meantime?

@xeno6696
Copy link
Collaborator

xeno6696 commented Nov 7, 2017

@rlevine1106

I double-checked the commits, it looks like we were able to do a clean version upgrade, with no failing unit tests. You should be alright if you override dependencies on your project.

@kwwall
Copy link
Contributor

kwwall commented Nov 7, 2017 via email

@kwwall
Copy link
Contributor

kwwall commented Nov 7, 2017 via email

@xeno6696
Copy link
Collaborator

xeno6696 commented Nov 7, 2017

@kwwall this one was already merged. All we have to do at this point is generate the docs and sally forth!

@Maarten-Damen
Copy link

@kwwall is it possible to release a 2.1.0.2 with the CVE fixes, like @augustd asked? Or are there insights on the progress of the bug fix you're working on?

@kwwall
Copy link
Contributor

kwwall commented Nov 10, 2017

We are working on a 2.1.0.2 release without most of the crypto-related fixes addressed. That's largely because I would like to find some crypto experts to provide feedback on what I've developed so far so we don't revisit that CVE road again in the future. Hope to have it out by the end of the year.

@kwwall
Copy link
Contributor

kwwall commented Nov 10, 2017

In the meantime, just pull in the newer versions of those libraries into your application's classpath. All the current ESAPI JUnit tests seem to pass with them, although you may also want to regression test your application with them as well in case we missed a critical test somewhere.

@deebansal
Copy link

Any update esapi 2.1.0.2 release ?

@kwwall
Copy link
Contributor

kwwall commented May 30, 2019

This is fixed in Maven Central as ESAPI 2.2.0.0-RC2. We will be pushing out a RC3 shortly and hopefully, by the end of June (2019, because, you know, it's been so long, I had to say that) the official 2.2.0.0 release.

@DAN-MU-ZI DAN-MU-ZI mentioned this issue Nov 4, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests