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 #2018

Closed
JLLeitschuh opened this issue Feb 19, 2019 · 19 comments

Comments

@JLLeitschuh
Copy link
Contributor

CWE-829: Inclusion of Functionality from Untrusted Control Sphere

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 exists already to maliciously compromise a JAR file inflight.
See:

MITM Attacks Increasingly Common

See:

Source Locations

The TestNG Official Website:
http://testng.org/doc/download.html

<repositories>
  <repository>
    <id>jcenter</id>
    <name>bintray</name>
    <url>http://jcenter.bintray.com</url>
  </repository>
</repositories>

There may be more locations on the site that need to be updated.

In the build source:

I don't know if the Kobalt build is also vulnerable, so if someone with more experience could check for that, that would be great.

Disclosure

A project maintainer for this project should probably file for a CVE number to inform the public about this vulnerability in the TestNG build.

If someone on this project is a CNA, please have them file it:
https://cve.mitre.org/cve/request_id.html

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

@krmahadevan
Copy link
Member

@JLLeitschuh - I am kind of trying to understand what is being expected out of this bug? The URL that you called out from the build.gradle file is basically used only when you are trying to build the TestNG codebase. Any project that consumes TestNG jar as a dependency is free to setup its repository urls (for e.g., via the settings.xml in the maven world) such that its https in nature (I know many companies resort to even setting up their own Nexus artifactory using which they resolve artifacts their company employee needs and which is https in nature)

@JLLeitschuh
Copy link
Contributor Author

@krmahadevan - This bug is to report that the build used to create the build artifacts that are published is vulnerable to this. The TestNG Gradle build is downloading source code over an untrusted channel (HTTP) and executing that code in the same environment where releases are produced. If a MITM has been achieved during the build of a release of TestNG, it could have been used to maliciously compromise the jar files that were published.

This is not something the user can avoid, the build that is used to produce this project's artifacts is fundamentally insecure.

@krmahadevan
Copy link
Member

@JLLeitschuh - Sure. I get that. Thanks for elaborating this.
But my question remains the same. What do you expect the TestNG codebase/dev team to be doing? I am more inclined to understand the outcome of this bug.

Are you suggesting that

  1. some code change is required (if yes, any pointers around that also would be great.. I get the part that we could merely toggle over to https endpoint, but I don't know if that is all that is needed)
  2. some sort of wiki page (or) documentation be created which explicitly calls this out ?

Or anything else.

I know you already mentioned

A project maintainer for this project should probably file for a CVE number to inform the public about this vulnerability in the TestNG build.

But what i would like to know is, how does this information trickle to a user? They still need to go and check somewhere about this to find out this information right.

Please bear with me if my questions are sounding naive, but the intent is for me to understand better as to what this all about.

@JLLeitschuh
Copy link
Contributor Author

some code change is required (if yes, any pointers around that also would be great.. I get the part that we could merely toggle over to https endpoint, but I don't know if that is all that is needed)

Yes, in the code base, that is all that is needed.

I also recommend that the team wipe their ~/.gradle directory from their personal machines as that might be compromised. The same should be done on whatever machine is used to perform releases.

But what I would like to know is, how does this information trickle to a user? They still need to go and check somewhere about this to find out this information right.

Reporting a security vulnerability to the CVE system will basically kick off letting interested parties know about this vulnerability. Some users won't care or won't be concerned & that's totally fine for some. Once a CVE is created and added to the CVE database that can be found here, it will also be picked up by NIST where they will add it to the National Vulnerability Database (NVD). Security auditing tools and companies can use this data on the CVE database or in the NVD to create reports that end up being rolled into security reporting software like Black Duck Open Hub, Sonatype Nexus Auditor and the Sonatype OSSIndex.

It might also be valuable to put a comment line in the TestNG Changelog.

Please bear with me if my questions are sounding naive, but the intent is for me to understand better as to what this all about.

Don't worry about it, thanks for asking!

The following information may also be of assistance, this is the new block of text I've been appending to my comments when I find this vulnerability in larger projects:

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.

@krmahadevan
Copy link
Member

ping @cbeust @juherr - Thoughts on how to take this forward ?

@juherr
Copy link
Member

juherr commented Feb 22, 2019

As dependencies are not included in the binary in Java. If they are corrupted, the TestNG binary is not.
I think MITM is only an issue for runtime and I don't see any reason to open a CVE.

But you're right and we can use https everywhere we can.

@JLLeitschuh
Copy link
Contributor Author

@juherr If, during your release process, before publishing your releases, you execute the Gradle test task you are vulnerable. If you are executing the test task prior to publishing, malicious code could have had the opportunity to maliciously compromise the artifacts of the build.

Looking at your .travis.yml file, it looks like, in the process of performing a release the tests are executed meaning that potentially malicious code could have been executed before the artifacts were uploaded.

https://github.com/cbeust/testng/blob/1082fa6e819fd9acf1f1a836448a61da3476b746/.travis.yml#L30-L34

@juherr
Copy link
Member

juherr commented Feb 22, 2019

Right, it could be. We will update url to https.

Notify us if you find corrupted binaries.

@JLLeitschuh
Copy link
Contributor Author

Unfortunately, I currently have the resources to inspect the binary releases for TestNG to see if they were compromised.

I'm dealing with this vulnerability existing in many projects across the industry. I'm currently in contact with the security teams for at least 4 different organizations.
If you want a small snapshot into the places I've been finding it take a look here:

https://github.com/issues?utf8=%E2%9C%93&q=author%3AJLLeitschuh+archived%3Afalse+sort%3Aupdated-desc+%5BSECURITY%5D

@JLLeitschuh
Copy link
Contributor Author

@cbeust The website needs an update, also, if you could update your site to support HTTPS, that would be awesome.

@JLLeitschuh
Copy link
Contributor Author

Here's a CVE issued by the Eclipse security team to report the potential for compromise of Eclipse hawkBit:

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-10240

Without some sort of audit, a similar CVE number will need to be filed for the TestNG project.

@JLLeitschuh
Copy link
Contributor Author

JLLeitschuh commented Apr 18, 2019

If there is no followup on this issue by June 1st regarding an audit, I'll file for the CVE number for all previous versions of TestNG.

I don't have the resources to perform an audit myself.

@JLLeitschuh
Copy link
Contributor Author

Friendly heads up that without an audit, a CVE number will be issued on June 1st, 2019.

@Jman420
Copy link

Jman420 commented Feb 18, 2020

Has this issue been resolved? This is a fairly serious issue from an Information Security perspective and is easily fixed by the development team by, as stated above, simply switching to HTTPS for their dependency feeds. I'm really not sure why this wouldn't have been done as soon as this issue was reported... HTTPS is pretty much the defacto standard for regular web communications... nothing should be going over unencrypted channels.

@JLLeitschuh
Copy link
Contributor Author

Looks like it may have been

@cbeust
Copy link
Collaborator

cbeust commented Feb 19, 2020

I just made a bunch of replacements http:// -> https:// in both TestNG and its documentation.

@JLLeitschuh
Copy link
Contributor Author

I've looked around and I agree. I think this is resolved.

@krmahadevan
Copy link
Member

@JLLeitschuh - In that case, can we go ahead and close this issue ?

@JLLeitschuh
Copy link
Contributor Author

Sure! Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants