False positive detection of a vulnerability that has been fixed

[AgustinBettati]

Problem statement
We have a PR check that is currently failing as it detects there is a vulnerability in the version that is being updated.
This however does not seem accurate, as the version of tj-actions/verify-changed-files is being bumped from 58f5ac78e19e6cc3fb9d4048ae1a13bf364fa983 to 5ef175f2fd84957530d0fdd1384a541069e403f2 (latest commit at the time), while the fix for the mentioned vulnerability (GHSA-ghm2-rq8q-wrhc) was fixed in a commit previous to both of these 2acec78834cc690f70b3445712363fc314224127.

Given that the pinned sha already has the fix I would expect to not have this vulnerability failure.

[febuiles]

@AgustinBettati Thank you for reporting this issue. After spending a bit of time trying to find out how vulnerabilities work in pinned SHAs, I don't think Dependency Review Action has a great answer to offer here. Suppose we have an Action with two commits:

1. Commit abcd introduces a vulnerability
2. Commit ef01 fixes the vulnerability

How do we know that version ef01 (or any version really) is more recent than the commit introducing the vulnerability (abcd)? With traditional versioning schemes like SemVer it's easy to tell that 2.0.0 is more recent than 1.0.0 because 2 is greater than 1. Because pinned SHAs do not have this property, we can't tell Action-side if abcd is greater or not than ef01.

I looked into the vulnerability ranges GitHub reports for this Action, and it reports that this vulnerability affects all versions < 17, so I'm guessing this works fine when using version numbers but not SHAs.

@jonjanego Pinning SHAs is one of the recommended security practices. Do you know if/how Dependency Graph or other supply chain products in GitHub do version comparisons against SHAs to find vulnerabilities?

[febuiles]

A bit more triage info:

$ gh api repos/future-funk/congenial-chainsaw/dependency-graph/compare/main...febuiles-patch-2
[
  {
    "change_type": "added",
    "manifest": ".github/workflows/updates.yml",
    "ecosystem": "actions",
    "name": "tj-actions/verify-changed-files",
    "version": "5ef175f2fd84957530d0fdd1384a541069e403f2",
    "package_url": "pkg:githubactions/tj-actions/verify-changed-files@5ef175f2fd84957530d0fdd1384a541069e403f2",
    "license": null,
    "source_repository_url": "https://github.com/tj-actions/verify-changed-files",
    "scope": "runtime",
    "vulnerabilities": [
      {
        "severity": "high",
        "advisory_ghsa_id": "GHSA-ghm2-rq8q-wrhc",
        "advisory_summary": "Potential Actions command injection in output filenames (GHSL-2023-275)",
        "advisory_url": "https://github.com/advisories/GHSA-ghm2-rq8q-wrhc"
      }
    ]
  },
  {
    "change_type": "removed",
    "manifest": ".github/workflows/updates.yml",
    "ecosystem": "actions",
    "name": "tj-actions/verify-changed-files",
    "version": "58f5ac78e19e6cc3fb9d4048ae1a13bf364fa983",
    "package_url": "pkg:githubactions/tj-actions/verify-changed-files@58f5ac78e19e6cc3fb9d4048ae1a13bf364fa983",
    "license": null,
    "source_repository_url": "https://github.com/tj-actions/verify-changed-files",
    "scope": "runtime",
    "vulnerabilities": []
  }
]

The DR API endpoint is wrongly reporting thattj-actions/verify-changed-files at version 5ef175f2fd84957530d0fdd1384a541069e403f2 is vulnerable for the GHSA (it's not). The VVR for this advisory (49560) has the fields fixed: 17, affects: < 17.

[AgustinBettati]

@febuiles thank you for the updates here.
From your latest update just wanted to be clear if this might be a bug associated to the specific action (tj-actions/verify-changed-files), or if dependency-review-action lacks support for pinned SHAs altogether and in that case we can transition this to an enhancement request.

[febuiles]

@AgustinBettati Sorry for the confusion. There's nothing wrong with the verify-changed-files Action, the issue here is with the GitHub Dependency Review API not being able to infer the proper ordering of pinned GitHub Actions versions. Keeping the bug tag since users should not have PRs blocked for non-vulnerable dependencies.