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

High severity issues in security dependency check - False Positive #73

Open
ghost opened this issue Sep 19, 2022 · 6 comments
Open

High severity issues in security dependency check - False Positive #73

ghost opened this issue Sep 19, 2022 · 6 comments
Labels
documentation Improvements or additions to documentation

Comments

@ghost
Copy link

ghost commented Sep 19, 2022

ktlint ruleset shows up as high severity issue when running a dependency check.
pinterest/ktlint#512

The reason is that the ktlint module packages as ktlint.jar and ktlint-core.jar, which results in a false positive because it thinks version 0.0.5 (also 0.0.12) is below ktlints fix version 0.30.0 if I am not mistaken.

To Reproduce
Run plugin org.owasp.dependencycheck 7.1.1 and see it fail

Expected behavior
It shouldn't show as high severity issue which is a false-positive

Additional information
core-ktlint-0.0.5.jar | NVDCVE-2019-1010260 | High | CWE-319
File Path /var/lib/jenkins/.gradle/caches/modules-2/files-2.1/com.twitter.compose.rules/core-ktlint/0.0.5/f9f346f5a1fd509f84e53775ec52f18514c4ee42/core-ktlint-0.0.5.jar
SHA-1 f9f346f5a1fd509f84e53775ec52f18514c4ee42
SHA-256 5c8976a039ecedeb10de5fa44b56e1014b71badbcd1404c89c9643221f173462
Description Using ktlint to download and execute custom rulesets can result in arbitrary code execution as the served jars can be compromised by a MITM. This attack is exploitable via Man in the Middle of the HTTP connection to the artifact servers. This vulnerability appears to have been fixed in 0.30.0 and later; after commit 5e547b287d6c260d328a2cb658dbe6b7a7ff2261.

ktlint-0.0.5.jar | NVDCVE-2019-1010260 | High | CWE-319
File Path | /var/lib/jenkins/.gradle/caches/modules-2/files-2.1/com.twitter.compose.rules/ktlint/0.0.5/7954c9ff6e47f94dce73bc6f534c22f66bdb34fb/ktlint-0.0.5.jar
SHA-1 7954c9ff6e47f94dce73bc6f534c22f66bdb34fb
SHA-256 7e57dc0e98863516afacac94b6ffdea50b6226e1fae5f280581da642b2c6d7b0
Description Using ktlint to download and execute custom rulesets can result in arbitrary code execution as the served jars can be compromised by a MITM. This attack is exploitable via Man in the Middle of the HTTP connection to the artifact servers. This vulnerability appears to have been fixed in 0.30.0 and later; after commit 5e547b287d6c260d328a2cb658dbe6b7a7ff2261.

@mrmans0n
Copy link
Contributor

I think this is a problem in the plugin flagging this library with the false positive? not sure what you want us to do here 🤷

@ghost
Copy link
Author

ghost commented Sep 19, 2022

A) I am submitting this issue to have awareness for other devs that run into this
B) I have marked it as a false-positive in our app, it should no longer be an issue when you go above 0.30.0 or rename the jar? But I don't think both are good solutions. But this brings me to A) which is to somewhat raise awareness?

@kenyee
Copy link
Contributor

kenyee commented Sep 19, 2022

sounds like we need to rename our artifacts....that depguard is looking at artifact names and ours collides w/ ktlints.

@mrmans0n
Copy link
Contributor

sounds like we need to rename our artifacts....that depguard is looking at artifact names and ours collides w/ ktlints.

Wouldn't this mean changing the coordinates, at least for the top level ones, com.twitter.compose.rules:(ktlint|detekt) ?
If this was about the uber jar, it would be doable, but I am not at all for changing the coordinates name 😓

FWIW I think this won't be an issue when we bump the major to 1.0.0 and keep going from there. I was planning to do this when things stabilized a bit, and it looks like we are getting close to that. So hopefully the issue @tialawllol reported won't be an issue anymore 😄

By the way, @tialawllol, do you mind sharing how your suppression looks like, in case anybody stumbles upon this issue? I can see the docs but I've never used dependency-check 😅

@kenyee
Copy link
Contributor

kenyee commented Sep 21, 2022

Wouldn't this mean changing the coordinates, at least for the top level ones, com.twitter.compose.rules:(ktlint|detekt)

Sadly yes and I agree... 😿

@ghost
Copy link
Author

ghost commented Sep 22, 2022

Well with just a version update it could still result in other false positives, depending on ktlint's version, but it's a step.

This is how our suppressions.xml currently looks for this issue

<?xml version="1.0" encoding="UTF-8"?>
<suppressions xmlns="https://jeremylong.github.io/DependencyCheck/dependency-suppression.1.3.xsd">
    <suppress>
        <notes><![CDATA[
        Wrong detection of core-ktlint library, see: https://github.com/twitter/compose-rules/issues/73
        ]]></notes>
        <sha1>8b163196c50e68a62e3b5bb910a99e8415889654</sha1>
        <cve>CVE-2019-1010260</cve>
    </suppress>
    <suppress>
        <notes><![CDATA[
        Wrong detection of ktlint ruleset library, see: https://github.com/twitter/compose-rules/issues/73
        ]]></notes>
        <sha1>de64d1b35289d73edac35724941de3099193f782</sha1>
        <cve>CVE-2019-1010260</cve>
    </suppress>
</suppressions>

@mrmans0n mrmans0n added the documentation Improvements or additions to documentation label Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants