Skip to content

Conversation

benjsmi
Copy link
Contributor

@benjsmi benjsmi commented Apr 16, 2024

As per #337

@benjsmi benjsmi requested a review from a team as a code owner April 16, 2024 15:44
Signed-off-by: Ben Smith <benjsmi@us.ibm.com>
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@denyeart denyeart merged commit 7f6cf32 into hyperledger:main Apr 16, 2024
@denyeart
Copy link
Contributor

@benjsmi
BTW if you use a term like "Resolves" instead of "As per" when referencing an issue, it will create a link and automatically close the issue when the pull request is merged. See the list of magic key words at:
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@denyeart
Copy link
Contributor

@benjsmi
And another BTW... if you know you are going to open a pull request, you don't even need to open a corresponding issue. You can just open the pull request. We typically open issues as a reminder for future work items.

@bestbeforetoday
Copy link
Member

bestbeforetoday commented Apr 16, 2024

I don't completely understand this change. CVE-2024-23080 is currently listed as Disputed for a vulnerability in joda-time v2.12.5. Before this change, the version of joda-time resolved by fabric-chaincode-shim was v2.10.2. After this change, the version of joda-time resolved is still v2.10.2. There is no vulnerability showing up in the scheduled vulnerability scan.

What does this change fix?

@benjsmi benjsmi deleted the joda-time branch April 17, 2024 13:02
@benjsmi
Copy link
Contributor Author

benjsmi commented Apr 17, 2024

Yeah we may want to reopen this issue (#337). I attempted to resolve this problem with my PR but it doesn't seem to have done the trick as @bestbeforetoday indicates.

Specifically, I was under the impression that moving to latest org.everit.json.schema (that is v1.14.4) was going to move us to latest handy-uri-templates, which would, in turn, move us to latest joda-time. It did not make the change.

It looks like the whole Everit JSON Schema project has moved from https://mvnrepository.com/artifact/com.github.erosb/everit-json-schema/1.14.4 over to https://mvnrepository.com/artifact/com.github.java-json-tools/json-schema-validator? Is it possible to use this in place of the other? everit-json-schema seems to have gone end of life.

The trace is:

everit-json-schema -> handy-uri-templates -> joda-time...

So, I think maybe we are more stuck than I thought, unless we are able to override transitive dependencies in Maven (I've never known how to do that). From where I sit, we'd need handy-uri-templates to move to a newer/later version of joda-time and then we'd need everit-json-schema to move to that newest version of handy-uri-templates.

Regarding the disputed nature of CVE-2024-23080... a few things: 1) the tools that I am required to use don't show it as disputed, even though I agree with you that it is. 2) it's easier to just move to the latest version than to just explain over and over again that it's a disputed CVE. 3) I've seen many CVEs be disputed until they just aren't, so it's probably a temporary thing.

@bestbeforetoday
Copy link
Member

I can totally relate to just updating the dependency as being the easiest path. I've been in the same situation many times.

Maven prefers the closest dependency in the dependency tree. This means a top-level POM file dependency on a component at a specific version will take precedence over the versions of any transitive dependencies on the same component. This is generally not a good idea since changes between versions can break the other components that depend on it at a specific version. If possible, it safer to update just the direct dependencies to versions where the transitive dependency is at a good version.

At a quick glance, it doesn't look like everit-json-schema leaks out into the public API anywhere so I'd be quite happy for it to replaced by something newer/better.

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

Successfully merging this pull request may close these issues.

3 participants