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] Releases are built/executed/released in the context of insecure/untrusted code #301

Closed
JLLeitschuh opened this issue Feb 26, 2019 · 10 comments

Comments

@JLLeitschuh
Copy link

CWE-829: Inclusion of Functionality from Untrusted Control Sphere
CWE-494: Download of Code Without Integrity Check

The build files indicate that this project is resolving dependencies over HTTP instead of HTTPS. Any of these artifacts could have been MITM to maliciously compromise them and infect the build artifacts that were produced. Additionally, if any of these JARs or other dependencies were compromised, any developers using these could continue to be infected past updating to fix this.

This vulnerability has a CVSS v3.0 Base Score of 8.1/10
https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:H/A:H

This isn't just theoretical

POC code has existed since 2014 to maliciously compromise a JAR file inflight.
See:

MITM Attacks Increasingly Common

See:

Source Locations

maven { url "http://repo.spring.io/plugins-release" }

@ben-manes
Copy link
Owner

Oh thanks. Note that this is only for the project's own build (plugins like JMH) and wouldn't impact user's, but definitely worth fixing.

@vlsi
Copy link
Contributor

vlsi commented Feb 26, 2019

@ben-manes , do you think it is worth applying something like https://github.com/signalapp/gradle-witness to validate the checksums of the downloaded artifacts?

@ben-manes
Copy link
Owner

There are already checksum files generated on the repositories that the dependency manager should verify with, so I don't think that plugin would help?

For example,
https://repo1.maven.org/maven2/com/github/ben-manes/caffeine/caffeine/2.7.0/

@vlsi
Copy link
Contributor

vlsi commented Feb 26, 2019

Eh, I should probably test it, but it was not clear if Gradle verifies them: https://discuss.gradle.org/t/is-there-an-option-to-validate-downloaded-artifacts-via-the-sha1-checksum/6054/3

@JLLeitschuh
Copy link
Author

Gradle does not verify checksums. They do use checksums to see if they need to pull an updated version from the repository.

This could impact users since any maliciously downloaded code is executing in the context where the releases are produced. The malicious code could have maliciously compromised the build artifacts before upload.

Public Disclosure

Option 1: File for a CVE

A project maintainer for this project should probably file for a CVE number to inform the public about this vulnerability in the build for this project. The goal is to inform the public that there was a potential for published build artifacts to have been maliciously compromised in earlier releases.

If a maintainer on this project works for or is associated with a CNA, please have them file it with them:
cve.mitre.org/cve/request_id.html

Otherwise, an open source CVE should be filed for here:
iwantacve.org

Option 2: Manually validate the release artifacts

If this project's build is fully reproducible. An alternative to filing for a CVE is to go back and build the earlier releases (with the HTTPS patch applied) to confirm the artifacts were not tampered when they were built. This can be done by comparing the hashes of the artifacts built locally with the ones published. If the hashes of all previous artifacts match those that are published, you can safely assume that the releases were not tampered with.

Again, this assumes that the build if fully reproducible and will require significantly more work.

@ben-manes
Copy link
Owner

The build is reproducible and everything appears to match.

@vlsi
Copy link
Contributor

vlsi commented Feb 27, 2019

There are already checksum files generated on the repositories that the dependency manager should verify with, so I don't think that plugin would help

Just in case: the checksums you list are located on the remote site, and they can be compromised.

So the proper way to approach it is to declare expected checksums and/or expected GPG keys at use side (e.g. how MacPorts , NixOS, Brew, and so on package managers declare expected sums).

In other words, Maven should enable users to write like:

<dependency>
   <artifactId>caffeine</artifactId>
   <version>2.7.0</version>
   <sha1>c3af06be4a7d4e769fce2cef5e77d3becad9818a</sha1>
</dependency>

Then, the downloaded file should be compared with user-provided checksum.
Unfortunately, Maven is not there yet.
Gradle does not have a notion to declare "expected checksums" either :(

I see Caffeine does not have much of the (runtime) dependencies, however every dependency matters.
For instance, a compromised Gradle Wrapper might do bad things, so you might want to add distributionSha256Sum=... to gradle-wrapper.properties just in case (e.g. https://github.com/vlsi/jmeter/blob/gradle/gradle/wrapper/gradle-wrapper.properties#L3 )

@ben-manes
Copy link
Owner

ben-manes commented Feb 27, 2019

Unfortunately distributionSha256Sum isn't generated with gradlew wrapper.

Maven's workaround for dependencies is the enforcer and pgpsigner plugins. Gradle had the witness plugin, but it's been abandoned since 2014 and appears to no longer work. Neither seem to have put much effort here.

Central, Spring, and jitpack repositories have good enforcement policies that make it easy to track the history of dependencies. The most unsafe is bintray since they allow stealing a coordinate and by their very lax rules. Sadly none seem to instruct the http clients to redirect to https.

So the ecosystem is in a bit of a bad state I guess.

@vlsi
Copy link
Contributor

vlsi commented Feb 27, 2019

Of course I meant gradle-witness, thanks for pointing that out.

@JLLeitschuh
Copy link
Author

JLLeitschuh commented Feb 27, 2019

The gradle-witness plugin works fine with Gradle 5.1 as evidenced by the Signal-Android build using it without issue with Gradle 5.1.

https://github.com/signalapp/Signal-Android

https://github.com/signalapp/gradle-witness

The build is reproducible and everything appears to match.

AWESOME! That takes the requirement to disclose off the table. Glad you are in the clear.

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

3 participants