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

security: bump snakeyaml dep to 2.0 #82

Closed
wants to merge 1 commit into from
Closed

Conversation

jmini
Copy link

@jmini jmini commented Apr 6, 2023

Fixes: #81

@abstractj abstractj mentioned this pull request Apr 6, 2023
2 tasks
@jmini
Copy link
Author

jmini commented Apr 7, 2023

This PR is a minimal change to eliminate the CVE-2022-1471, but in my opinion it would be better to remove the runtime dependency as I did with #83

@davideberlein
Copy link

@bpossolo can we please have this merged and released? This is causing critical vulnerability alerts on this lib

@abstractj
Copy link

@mattrobenolt could you please review those changes so we can eliminate a CVE?

@mattrobenolt
Copy link
Member

@mattrobenolt could you please review those changes so we can eliminate a CVE?

What. I don't know Java. Not sure why I got pinged for this.

@orgads
Copy link

orgads commented Jul 6, 2023

Can anyone please approve this?

@olamy
Copy link

olamy commented Aug 3, 2023

done in a fork and released here https://github.com/olamy/uap-java

@dcolazin
Copy link

dcolazin commented Oct 6, 2023

Seems that with the version bump, there is no deprecation of the methods used, and the tests are passing. The only concern might be a performance benchmark between the old and the new version. Anyway, even this should be an quick and easy check.

I will be blunt. I don't understand why such a simple fix of a critical/high cve in a widely used library (we can discuss if the score is accurate, but marking this as fp is not worth the time to discuss with security teams) is taking so long. This project needs more maintainers! @mattrobenolt you were pinged probably because you appear as the sole person in the ua-parser organization.

Yes, it is possible to do something like the following, but some vulnerability checkers will also read the uap-java pom and still consider it vulnerable.

<dependency>
	<groupId>com.github.ua-parser</groupId>
	<artifactId>uap-java</artifactId>
	<exclusions>
		<exclusion>
			<groupId>org.yaml</groupId>
			<artifactId>snakeyaml</artifactId>
		</exclusion>
	</exclusions>
</dependency>
<dependency>
	<groupId>org.yaml</groupId>
	<artifactId>snakeyaml</artifactId>
	<version>2.0</version>
</dependency>

@jmini
Copy link
Author

jmini commented Nov 14, 2023

The only concern might be a performance benchmark between the old and the new version.

About performance:
We are now using a SnakeYaml free version since months now.
The yaml is converted to Java code at build time.
See #83 (comment)

@dcolazin
Copy link

dcolazin commented Nov 16, 2023

About performance: We are now using a SnakeYaml free version since months now. The yaml is converted to Java code at build time. See #83 (comment)

It would be great to have a bump fix or a radical fix like yours in an official release.

@bpossolo could you check this issue?

@bpossolo
Copy link
Contributor

snakeyml has been updated to 2.2 and is included in uap-java release 1.6.0

@bpossolo bpossolo closed this Nov 28, 2023
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.

CVE-2022-1471 - Dependency Snakeyaml Critical 9.8 score vulnerability
9 participants