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

Update org.eclipse.xtend.lib dependency to 2.28.0 or later to address CVE-2020-8908 #672

Closed
cherylking opened this issue Oct 11, 2022 · 12 comments · Fixed by #673
Closed
Milestone

Comments

@cherylking
Copy link
Contributor

cherylking commented Oct 11, 2022

Our language server is currently using org.eclipse.lsp4j:0.14.0 which ultimately depends on guava:27.1. The low severity vulnerability CVE-2020-8908 has been detected by our Mend scan and indicates that guava:30.0 or later must be used.

Screen Shot 2022-10-11 at 2 13 52 PM

The org.eclipse.xtend.lib:2.28.0 and org.eclipse.xtext.xbase:2.28.0 update the guava version to 30.1 and would resolve this issue. Please consider updating the lsp4j artifacts to use a minimum of the 2.28.0 version of org.eclipse.xtend.lib. Thank you for any attention to this matter.

@jonahgraham jonahgraham added this to the 0.17.0 milestone Oct 11, 2022
@jonahgraham
Copy link
Contributor

Hi @cherylking Thank you for the report. #529 has been trying to break the dependency on xbase.lib for a while. I think that would resolve this issue. However that has been ongoing for a while. It would be great if you are able to help review that change so we can break the dependency altogether.

In the meantime, can you provide a PR to make that update as it looks like you have already done most of the analysis work required to get us there. I think the version is only defined here: https://github.com/eclipse/lsp4j/blob/f84db765ce5f232c899fe48b398a8eb6d67e4c1e/gradle/versions.gradle#L16

We can do a release 0.17.0 if needed.

BTW. a few things about this:

  1. Your report is against 0.14.0, I think the same issue in 0.16.0 that we released last week in https://github.com/eclipse/lsp4j/releases/tag/v0.16.0
  2. The dependency tree you have provided is for the lsp4j.generator, that code isn't needed except to build LSP4J, i.e. it shouldn't be in installed for the user. Even if we finish [#528] remove runtime dependencies to xbase.lib #529 I think xbase will still be used.
  3. LSP4J allows Guava 30: https://github.com/eclipse/lsp4j/blob/f84db765ce5f232c899fe48b398a8eb6d67e4c1e/gradle/versions.gradle#L17-L18 does xbase.lib have a hard dependency on the version? I didn't think it did and you should be able to use the newer guava with no LSP4J changes. That said, I am much more familiar with the OSGi dependencies than the maven ones, so what works with OSGi may not work with maven.

@cherylking
Copy link
Contributor Author

cherylking commented Oct 11, 2022

@jonahgraham The xbase.lib lists a compile dependency on guava with no version in the pom.xml. It uses a bom for dependency management. That bom lists the following:

      <dependency>
        <groupId>com.google.guava</groupId>
        <artifactId>guava</artifactId>
        <version>27.1-jre</version>
        <scope>compile</scope>
      </dependency>

So it seems like a hard dependency.

To address your feedback above:

  1. Yes, the same issue exists with 0.16.0.
  2. Not sure there is a way to tell Mend that the dependency is not used outside the build.
  3. Are you suggesting I could add something to my pom.xml to update guava to 30.0 or later and that would override what was specified in the transitive dependency? I presume this is not the case since xbase.lib listed a hard dependency on the version.

As for reviewing the existing PR that removes the dependency, I don't have enough familiarity with lsp4j at this point.

Regarding my creating a PR to use org.eclipse.xtend.lib:2.28.0 or later, I presumed that change might not be trivial and could require code changes. Would you like me to go ahead and create a PR with that change and see if there are complications?

@nixel2007
Copy link
Contributor

Are you suggesting I could add something to my pom.xml to update guava to 30.0 or later and that would override what was specified in the transitive dependency?

Yes, because you can't have 2 different versions of the same dependency (without shadowing or package-rewrite hacks), your concrete version in top pom.xml will overwrite version in xtext bom.

mvn has a goal for dependencies list print (gradle has it either), you should be able to check it after pom.xml change.

@jonahgraham
Copy link
Contributor

If @nixel2007 isn't enough to resolve your issue, then please do go ahead and make the PR. That will run the tests and do some API analysis to make sure everything is working as expected.

@szarnekow
Copy link

So it seems like a hard dependency.

Please note that this is only the dependency that we build and test against. From my experience, xbase.lib works fine with a more recent Guava version. You can override the version of that dependency in your local build for the time being.

@cherylking
Copy link
Contributor Author

@jonahgraham I created a fork of this repo and updated the org.eclipse.xtend.lib dependency to 2.28.0 and the local gradle build failed. Without the change, it was successful. So I don't think it is a straight forward update.

Screen Shot 2022-10-12 at 4 01 43 PM

@jonahgraham
Copy link
Contributor

Thanks @cherylking for the info.

I see that you have updated your code so that you aren't blocked here? OpenLiberty/liberty-language-server#122

I am not sure when someone on here will get to looking at this. If you do figure it out why it failed, we would be grateful for a PR.

@cherylking
Copy link
Contributor Author

@jonahgraham I am not blocked currently, but we need to do some testing to make sure my overriding that transitive dependency doesn't break anything. I would like to keep this issue open as it seems like updating to a more recent org.eclipse.xtend.lib dependency would be wise. I am not familiar with the lsp4j code base though, and could not even find the test case that I got a failure on.

Just wanted to post with my findings above so that you are aware it will take some investigation (that I unfortunately do not have time to do myself at this point). Thanks for your guidance and attention to this issue.

@cdietrich
Copy link
Contributor

@oehme any idea?

@cdietrich
Copy link
Contributor

cdietrich commented Oct 13, 2022

might becaused by this https://github.com/eclipse/lsp4j/blob/5605ca82252833777dbef82ca13a403dbfb9d906/gradle/java-compiler-settings.gradle#L84

configurations.all {
	resolutionStrategy {
		force "com.google.guava:guava:18.0"
	}
}

i also assume that this one is no longer needed
https://github.com/eclipse/lsp4j/blob/f84db765ce5f232c899fe48b398a8eb6d67e4c1e/build.gradle#L50

@oehme
Copy link

oehme commented Oct 13, 2022

@cdietrich yes, if you wanna upgrade xtend.lib, you need to remove (or update) that force rule for guava

@cherylking
Copy link
Contributor Author

configurations.all {
	resolutionStrategy {
		force "com.google.guava:guava:18.0"
	}
}

@cdietrich Removing that fixed it. Thanks for the pointer. I'll get a PR created later today. I wasn't sure about the other change you listed. The build was successful without changing that "Tooling" section in build.gradle.

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 a pull request may close this issue.

6 participants