search

DependencyTrack / dependency-track

Dependency-Track is an intelligent Component Analysis platform that allows organizations to identify and reduce risk in the software supply chain.
https://dependencytrack.org/
Apache License 2.0
2.69k stars 578 forks source link

Add Packagist security advisories parser #798

Open Szasza opened 4 years ago

Szasza commented 4 years ago

The Current Behaviour section assumes that https://github.com/DependencyTrack/dependency-track/pull/796 is already merged.

Current Behaviour:

When a BOM is parsed which contains Composer-type packages, there is no check if there is a reported vulnerability for the given package versions in Packagist.

Proposed Behaviour:

As per https://packagist.org/apidoc#list-security-advisories (bottom of the page) Packagist exposes an API endpoint which could be used to check if there are any security advisories for the packages in the BOM. This API could be utilised similarly to the NPM audit advisories which is already implemented. The difference to NPM is that specific package names need to be used with Packagist, so the security advisories would only be pulled when a BOM/dependency is added.

Additional note:

This is already being implemented but a PR will only be opened when https://github.com/DependencyTrack/dependency-track/pull/796 actually gets merged.

stevespringett commented 4 years ago

DT should use OSS Index (if enabled) for the analysis of PHP Composer components. I said should because I've never actually tested it. But the logic is that if a component has a PURL, the component will be analyzed with OSS Index.

But agree, this would be a nice enhancement to have.

Let's target all the PHP enhancements for 4.1.

Szasza commented 4 years ago

@stevespringett thank you for the information, it is much appreciated. I will check if the OSS Index is used for Composer components.

nscuro commented 4 years ago

@Szasza @stevespringett It is, my org already had a few findings pop up in the past. It would be interesting to know though if OSS Index already includes the advisories from packagist.

stevespringett commented 4 years ago

Good. Yes, regardless, I think having more analyzers is a good thing as it gives the community choice and reduces the reliance on a single source.

stevespringett commented 3 years ago

Moving this out from 4.1. Will target a future release.

One issue I see with supporting Packagist advisories is they all lack severity. If the advisory contains a CVE, the severity can be retrieved from that, but if the advisory doesn't contain a CVE, severity will be unknown.

jnkowa-gfk commented 3 years ago

really love to see this feature.

example: currently seeing a lot of issues with https://packagist.org/packages/typo3/cms-core but DependencyTrack does not show the issue, though an affected version of typo3/cms-core is listed.

Security API: https://packagist.org/api/security-advisories/?packages[]=typo3/cms-core

jkowalleck commented 3 years ago

Hello @Szasza ,

you wrote

This is already being implemented but a PR will only be opened when #796 actually gets merged.

796 was merged.

Did I understand you right, that the code for this very issue was created, but the PR is not opened, yet? Could you have a PR opened and linked to #798?

appreciate your work very much.

If SBOM generated by cyclonedx-php-composer needs to include additional information, feel free to reach out to me or open an issue for cyclonedx-php-composer, As a maintainer of cyclonedx-php-composer i will be happy to have it sorted out and implemented ASAP. :-)

jkowalleck commented 3 years ago

@stevespringett wrote in https://github.com/DependencyTrack/dependency-track/issues/798#issuecomment-766423800

One issue I see with supporting Packagist advisories is they all lack severity. If the advisory contains a CVE, the severity can be retrieved from that, but if the advisory doesn't contain a CVE, severity will be unknown.

Questions:

example for CVE in API response

stevespringett commented 3 years ago

DT supports severity of critical, high, medium, low, info, and unassigned. Since Packagist advisories do not support severity, then for advisories with a CVE the severity can be derived from the CVE. For advisories without a CVE, the severity will be unassigned.

Unassigned severity has the same risk score as high severity.

The UI will not be impacted. It already supports unassigned severity.

jkowalleck commented 3 years ago

Hello @stevespringett,

any updates on this feature? I assume not, since the situation with the data source seams to be unchanged.

tsfoer commented 3 years ago

Any updates on this?

jkowalleck commented 3 years ago

@Szasza could you add a PR for this?

796 was merged and you mentioned to have code ready, that required this merge.

jkowalleck commented 3 years ago

feature request: Strip v version prefix for packagist Index analysis (like #1220)

background & examples: see also: https://github.com/CycloneDX/cyclonedx-php-composer/issues/102 see package typo3/cms-core for example.

conclusion: the implementation should strip the v. dont rely on any SBoM sources to have the v stripped already.

JorisVanEijden commented 2 years ago

conclusion: the implementation should strip the v. dont rely on any SBoM sources to have the v stripped already.

The "affectedVersions" returned from that API is not a list of versions but a "composer constraint". To see if a version from an SBOM is vulnerable you have to perform the same composer matching logic. see https://github.com/composer/semver/blob/main/src/Semver.php