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.

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

Vulnerability found on com.ibm.icu.icu4j dependency #48

Closed
jzimermann opened this issue Nov 7, 2017 · 8 comments
Closed

Vulnerability found on com.ibm.icu.icu4j dependency #48

jzimermann opened this issue Nov 7, 2017 · 8 comments

Comments

@jzimermann
Copy link

The issue

Slugify library recently added a dependency called com.ibm.icu.icu4j as part of the effort to solve #45 and support other languages.

It was found here version 59.1 has a vulnerability that allows remote attackers to execute arbitrary code via a crafted string, aka a "redundant UVector entry clean up function call" issue.
We are working on a Pull Request that bumps the com.ibm.icu.icu4j version to 60.1 which should fix this.

@oloo pairing with me on this issue

@srl295
Copy link

srl295 commented Nov 9, 2017

@jzimermann the CVE is for icu for C not J. in anyway, the "remote code execution" is questionable, I think it's actually just a crash after memory allocation fails instead of an error result (and then a crash!)

@jzimermann
Copy link
Author

@srl295 thanks for the clatification. Would be amazing though if the version get updated to 60.1 and the PR #49 merged.

@oloo
Copy link

oloo commented Nov 10, 2017

Yes @srl295 , we did notice that the details in the CVE description referred to icu4c and not icu4j. To give you some background information. We are using spring-boot with owasp dependency checker as part of our build pipeline. The error we are getting is, icu4j-59.1.jar (com.ibm.icu:icu4j:59.1, cpe:/a:icu_project:international_components_for_unicode:59.1) : CVE-2017-14952 The CVE listed describes icu4c instead.
This could be as a result of one of these.

  1. The same vulnerability exists for both icu4j and icu4c either using shared code with the Java calling the C++ code (using JNI or something of that sort) or the similarity in implementations resulted in both libraries having that vulnerability.
  2. The dependency check gradle plugin has a bug which results in mistakenly flagging icu4j as having that vulnerability
  3. The folks maintaining the National Vulnerability Database mistakenly flagged icu4j for an icu4c vulnerability

I haven't explored any of these in detail, either way, it would be great if we moved to version 60.1 .

@srl295
Copy link

srl295 commented Nov 10, 2017

Thanks for your reply, @oloo .

  1. The same vulnerability exists for both icu4j and icu4c either using shared code with the Java calling the C++ code (using JNI or something of that sort) or the similarity in implementations resulted in both libraries having that vulnerability.

No, ICU4J does not call ICU4C, nor does it have similar implementation in this case. The UVector class is C++ only.

  1. The dependency check gradle plugin has a bug

I think the dependency checker may be buggy if it flagged an error. The CVE specifically mentions C++. Would you be able to file an issue and mention me?

I think one issue may be that the CPE cpe:/a:icu_project:international_components_for_unicode:59.1 doesn't mention C or Java. They should be split.

@oloo
Copy link

oloo commented Nov 10, 2017

@srl295, @jzimermann I have filed the issue here and tagged both of you.

@jeremylong
Copy link

@oloo for point 3 above - I'm not sure I would blame MITRE/NVD for mistakenly flagging icu4j as icu4c. OWASP dependency-check uses the data from the NVD to do a best effort matching - false positives do crop up from time to time.

For the maintainers of slugify - this issue can likely be closed. I will include some level of suppression rules in the next release of dependency-check so that this doesn't get incorrectly flagged in the future.

@oloo
Copy link

oloo commented Nov 11, 2017

Thanks for the clarification @jeremylong. In the meantime, we will add icu4j to our suppressions list while we wait for the next dependency-check release. Thanks again @srl295 , @jeremylong , @jzimermann

@dtrunk90
Copy link
Member

Version 2.2 is released now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants