Closed archanakeshireddy closed 4 months ago
Please test the fix posted at https://github.com/gwtproject/gwt/pull/9785 - it is deployed to a maven server, so you can verify the fixed jar and scan one. Lots of people have reported this issue, but no one has tested that the fix stops scanners from alerting about this, so we haven't merged the fix yet.
Thank you for the fix @niloc132 - will test and see if it passes our SonarQube scan.
Apologies for the naive question - is the vulnerability still there with this fix just hidden or was it a false positive? If so, why? Will check the other Issues posted against this repository but would appreciate your advice/feedback.
Its a reasonable question, but we have to get a bit philosophical, so forgive my answer being a little too self-indulgent. For a shorter answer, see https://github.com/gwtproject/gwt/issues/9659#issuecomment-481602218.
What is a security vulnerability? Is it enough to have a line of code, that if someone calls it wrong, could permit a denial of service like this one? Or does someone have to call that line for it to matter?
I'd suggest that there are many such lines of code that could be called in Java itself, like ByteBuffer.allocate(bignumber) or something else that might take a long time or consume a lot of resources. But while protobuf-java is considered to have a vulnerability here, why isn't Java itself?
The difference is that protobuf-java's vulnerability is typically exposed to untrusted network calls - any client using grpc or handling protobuf over rest etc can send malicious payloads that consume extra resources. GWT however doesn't use protobuf-java for network traffic, only as a tool to help read sourcemaps from disk (Thomas's linked answer above suggests that the streaming html parser also can use this, reading trusted HTML strings on the server, but I can't find a source for that). So, if the attacker has managed to change the contents of your war file or deployed server resources, your server would still be vulnerable - but then again, if they can change files like that, there are so many other attacks that you are vulnerable to as well.
Finally, can GWT's specific copy of protobuf-java be used for anything except reading sourcemaps from disk? If so, someone would have had to do so deliberately, as the packages have been renamed to avoid accidental reuse or replacing a "real" copy of protobuf-java on the classpath. Any generated code created by protoc will not uses these classes, so normal protobuf deserialization cannot be affected by this.
So, this should be classified as a false positive, unless someone has gone out of their way to use GWT's com.google.gwt.dev.protobuf
packages instead of the proper com.google.protobuf
types.
Thank you for the considered response @niloc132 - appreciate that time that you took to compose that and deliberate over my question. I agree on all counts - it's there, it's latent but there's no code that would trigger it without a change to the code base via a hacker. And frankly if they are already in the OS I can imagine many more dangerous things they could do immediately.
Yours along with @tbroyer's summary I think give me what I need. Security scanning tools are a blunt instrument and I guess in many cases they need to be but they don't take into account the actual "usage" of library.
Hi @ajgbarnes any feedback from your testing?
Thanks for the nudge - let me check with the team :)
One more bump, I'm going to start getting a little more aggressive about open and untested PRs so we can talk about cutting a release. I wouldn't want to skip this ticket, lots of people have told us it is important, but in two months no one has managed to download a jar to find out if it is fixed for them.
Coincidentally I have been looking at it this week. There's a line in our February scan that still shows it but I'm checking with the team to ensure the old version did not creep back into the code base. Will report back shortly.
OK so in the end we edited our own version of the file to remove the pom.xml (we did not use the PR version). We will test against your version too. If the fix is the same i.e. pom.xml removal, then it still appears in the scan but we can treat it as a false positive.
@ajgbarnes If possible could you share what your vulnerability scanner is keying off of to to detect the presence of that dependency? With the pom removed, and with the packages all changed, I'm not sure what is left that it could reasonably detect, even if it directly tried to use the code (since it would be in the wrong package).
That said, I am seeing that the version that was supposed to be deployed to our maven server without this pom still seems to have it. I can't guess why just yet, either another packaging step that re-adds it (the maven/ scripts do some jar re-arranging at least...), or if the PR didn't work as expected (though I am pretty sure we verified the SDK build at least. I'll take a closer look soon.
I'm not sure what it's keying off of, one of the tools we use is AquaSec - https://catalog.redhat.com/software/operators/detail/62ed2466c050c35b1a47213c
Seems our manual fix is ok for e.g. SonarQube but we must have the old library also somewhere else in the build image we're using so it might be ok when we remove that. Will report back but may be next week by the time we get there.
And thanks for checking the pom.xml on the PR. I would like to switch to that rather than our own custom version.
@ajgbarnes I've just pushed another build for this that should resolve this. It is re-deployed at the same repo with the same version, see https://repo.vertispan.com/gwt-snapshot/org/gwtproject/gwt-servlet/2.11.0-fix-9778-SNAPSHOT/
You can verify that the new snapshot (with the -2 suffix) deployed there no longer has that directory by hand by navigating the jar's contents at https://repo.vertispan.com/webapp/#/artifacts/browse/tree/General/gwt-snapshot/org/gwtproject/gwt-servlet/2.11.0-fix-9778-SNAPSHOT Before:
After:
@ajgbarnes I've just pushed another build for this that should resolve this. It is re-deployed at the same repo with the same version, see https://repo.vertispan.com/gwt-snapshot/org/gwtproject/gwt-servlet/2.11.0-fix-9778-SNAPSHOT/
You can verify that the new snapshot (with the -2 suffix) deployed there no longer has that directory by hand by navigating the jar's contents at https://repo.vertispan.com/webapp/#/artifacts/browse/tree/General/gwt-snapshot/org/gwtproject/gwt-servlet/2.11.0-fix-9778-SNAPSHOT Before:
After:
Hi @niloc132 what would be the license terms of using this ? Is it still governed by Apache v2 ?
@prashanthga yes, the changes are only to the build itself, no changes in the source, and the resulting artifact is apache v2 licensed.
Note that the linked PR will be closed and instead we will resolve this for GWT 2.12 by merging #9936, which will resolve it by removing the dependency entirely.
@prashanthga yes, the changes are only to the build itself, no changes in the source, and the resulting artifact is apache v2 licensed.
Note that the linked PR will be closed and instead we will resolve this for GWT 2.12 by merging #9936, which will resolve it by removing the dependency entirely.
Okay, when do we expect a release of GWT 2.12 ?
@prashanthga This is an open source project, contributed to by volunteers. Vertispan is a company which I co-lead and work under that will accept money to work on this kind of thing, but otherwise we rely on volunteered time and effort to create, review, and test changes to the project. As such, barring someone who makes it a priority for the community "it'll ship when its ready."
If this were a real security issue, or it were tested by anyone before the last release, it would already be released - however, this is a false positive, where anyone concerned can read this issue (and the linked discussions), understand what is going on, and update their own internal processes to reflect this.
If you're interested in speeding along the release, please consider helping to review open PRs, helping fix open bugs, volunteering for testing (when announced on the mailing list), contributing at https://opencollective.com/gwt-project, or contacting me for paid support.
**GWT version:2.8.2 and 2.10.0
Description
Our dependancy check report is reporting CVE for our project using gwt, because gwt-servlet depends on protobuf-java 2.5.0.
I found similar issue https://github.com/gwtproject/gwt/issues/9749 Is it safe to assume that gwt-servlet does not use google protobuf java.
I saw an another issue https://github.com/gwtproject/gwt/issues/9752I where plan is remove the proto-buf pom.xml from META-INF folder. Is it safe for us if we remove this pom.xml and recompile gwt so our dependancy check report does not report this issue.