jeremylong / DependencyCheck

OWASP dependency-check is a software composition analysis utility that detects publicly disclosed vulnerabilities in application dependencies.
https://owasp.org/www-project-dependency-check/
Apache License 2.0
6.37k stars 1.27k forks source link

Enhance swift analyzer to be more git, swift, and OS aware #5729

Open michalszelagsonos opened 1 year ago

michalszelagsonos commented 1 year ago

Swift scanner is producing a lot of false positives due to it not having awareness of git version, swift version, which OS it is running on, and whether the dependency is hosted on GH enterprise or not. For example, consider a project that is using swift-collections@1.0.4 as a dependency while using latest swift 5.8 on MacOS. The scan will produce 5 false positives, at the time of this writing, for this dependency alone:

All of them are related to old versions of swift or git client or distinctions between Linux and other OSes. Now, imagine a larger project that will have many such dependencies. The signal to noise ratio becomes very poor out of the gate as the user will be confronted with 50+ CVEs to review. When I onboarded a project recently, that was my experience. Normally I would be hesitant to suggest that the scanner be that aware of the environment but given how tightly integrated swift is with git and how many CVEs seem to be popping up on Linux alone, I think there is a strong case here to consider enhancing the scanner to improve its efficacy.

Another source of many CVEs is a possibility that dependencies may be coming from GH Enterprise server. I got a lot of CVEs that I had to suppress for dependencies hosted on GitHub. This one may also be worth considering as an enhancement since dependencies hosted on github.com can be safely assumed to not be coming from GH Enterpise.

What I would like to see if for the scanner to look at git client, swift version, OS, and be a little smarter about URLs to exclude a marge number of false positives.

aikebah commented 1 year ago

What you are looking for is an entirely different way of analysis than what DependencyCheck is built to do.

DependencyCheck (the core) uses text-matching to try and match up information extracted from dependencies (evidences) with the product-, vendor- and version coordinates of the NVD CPE.

For dependencies with 'swift' in there name, especially when they themselves have never seen a vulnerability chances are significant that the text-matching will match it up with swift itself (as it is the 'best' match that it can make with the CPEs that appear in NISTs NVD datafeed).

The analyzer's task is the extraction of such evidences. The core will do the matching up with CPEs, and for some cases will fail, which is why the CPE suppression mechanism is available

michalszelagsonos commented 1 year ago

I agree that what I am proposing is an extension of how the Dependency Check works. Perhaps there's a simple way to improve the scanner efficacy. If I could supply an option, say --swift-version 5.8 to influence the behavior, that would help a lot. Same case for git client. I understand that asking the tool to examine the environment is a slippery slope, but what if the user could provide additional "hints" to improve the results?. Taking CVE-2018-4220 as an example, knowing the context is 5.8 would not match the pattern, since the CVE references up to 4.1.1. I think I am arriving at a feature request to allow user to provide hints to improve results.

aikebah commented 1 year ago

Well... the build context is irrelevant for the purpose of dependendy-check (checking your project's dependencies for vulnerabilitie). If the project would depend on a vulnerable Swift version it would be vulnerable, if the project doesn't depend on it it would not be (though your build context might be vulnerable due to an out-of-date Swift framework).

What needs to be prevented is dependencies that are not Swift itself getting flagged as being the Swift CPE.

Not sure exactly what kind of condition would be good for Swift projects to capture 'this is actually the swift CPE itself', but typically such an expression would negated and then included in dependencycheck in the form

<suppression>
   <... regex="true">$negated-regex-that-would-identify-the-swift-framework</...>
   <cpe>cpe:/a:apple:swift</cpe>
<suppression>

To ensure that the CPE-Analyzer does not link the 'this is not Swift itself' dependencies to the Swift CPE.

Not sure what you mean by the git version. Unless the project actually depends on git as a library git version is completely irrelevant.

michalszelagsonos commented 1 year ago

Not sure I agree with build context being irrelevant. Perhaps I misunderstood that statement so here is what I see. The tools used in the build do matter as they are part of the overall process and can be exploited. Aside from Swift, Dependency Check does care about the build context in other tech stacks, for example gradle, where various build specific configurations are scanned for CVEs that aren't part of the target software (although user can disable that). I think this behavior is consistent and aligned with DC's goals and adds value.

Coming back to git client version, swift package manager uses it to retrieve dependencies. This is a very typical use case in swift. Example of a dependency declaration in Package.swift:

dependencies: [
        .package(url: "git@github.com:Some-Org/some-swift-package.git")
]

This triggers several CVEs related to git client, even though the project does not directly depend on it, but it is part of the build. The git client versions are rather old and likely not relevant to modern swift projects, from my suppression file:

<cve>CVE-2022-25648</cve>     <!-- git 1.11 command injection, not applicable-->
<cve>CVE-2020-10518</cve>     <!-- GitHub Enterprise remote code execution, not applicable-->
<cve>CVE-2020-10519</cve>     <!-- GitHub Enterprise remote code execution, not applicable-->
<cve>CVE-2020-5260</cve>      <!-- git 2.17 credentials disclosure, not applicable -->
<cve>CVE-2020-10517</cve>     <!-- GitHub Enterprise Server access control, not applicable -->

Is this a show stopper? Not really. I can add a bunch of these, 50+ overall to the file, and then go and do the same for the other 10 GitHub repos that are part of the project. From user's perspective, this is sub optimal and an area for improvement: the quality of the feedback is poor, large number of false positives adds lots of friction during adoption. If user were allowed to provide more info about the build context, this would drastically improve the accuracy. In my environment, it would be easy for me to programmatically check which versions of the relevant tools I am using and feed them into the scanner. This would work in lock step as I perform upgrades and not require a lot of care and feeding over time. I would definitely welcome this.

aikebah commented 1 year ago

The git issue I fully agree should be tackled in some way by DependencyCheck. It should not match the git product as a deoendency because of the presence of some git-url.

The build environment as such is certainly relevant to the security of your software delivery pipeline, no doubt about that. The 'irrelevant' to DependencyCheck is only in the sense that DependencyCheck is purposed to investigate on vulnerabilities in the components of your software. In that sense it is build-environment agnostic (unless your software product is itself the build-environment, in which case Swift would typically a component that you package as a part of it).

To my limited knowledge, but please correct me if I'm wrong on that, Swift version in the context of project definition is merely defining the framework API expected to be present on the targeted environments for the application, and swift version itself is maintained separately on the operating system of the user (as a standard part of the operating system and patched with the operating system patches). Within a project to my understanding it is only used to define the target-version that a deployment environment should be compatible with.

In my understanding targeting a project for e.g. Swift 5 would be similar to stating 'this project is targeting Java 11 deployments' in a Java project (an environment I'm more familiar with) - that is: defining the baseline API version that the deployment environment will able to provide. One would not consider the project to be subject to the vulnerabilities of a Java 11, but expect the end-user to ensure that a fully patched Java 11 compatible environment is provisioned on the deployment environment (unless the project contains a bundled version of a Java 11 VM)

In the gradle case: In my view the analogy would be that dependencyCheck would flag the gradle build tool and/or the JVM on which it runs as vulnerable, both of which are not the case. Gradle plugin dependencies are optionally supported recently (switched off by default), don't know whether there would be something similar in the Swift ecosystem, but gradle itself and the JVM are not scanned and those would in my perception be the counterpart of Swift version of a project.

michalszelagsonos commented 1 year ago

Yes, your understanding of the swift API version is correct. Thinking about this some more, in Package.swift file, which defines the project dependencies and versions, the target API version is also set (the very first line that looks like a comment but it isn't). Here is an example:

// swift-tools-version:5.8

import PackageDescription

let package = Package(
    name: "Foo",
    platforms: [
        .iOS(.v16),
        .macOS(.v13),
    ],
    products: [
        ....
    ],
    dependencies: [
        ...
    ],
    targets: [
        ...
    ]
)

The scanner support scanning this file. Given that target API is set in it, the scanner should have enough info to understand the target API level and ideally not present issues for swift 5.6, when the target version is 5.8 (using my example).