search

twitter / compose-rules

Static checks to aid with a healthy adoption of Compose
https://twitter.github.io/compose-rules
Other
1.36k stars 93 forks source link

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

Open ghost opened 2 years ago

ghost commented 2 years ago

ktlint ruleset shows up as high severity issue when running a dependency check. https://github.com/pinterest/ktlint/issues/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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

mrmans0n commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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>