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

information missing in npm7 audit report #45

Closed
valentijnscholten opened this issue Nov 2, 2020 · 8 comments
Closed

information missing in npm7 audit report #45

valentijnscholten opened this issue Nov 2, 2020 · 8 comments

Comments

@valentijnscholten
Copy link

What / Why

We're using the npm audit json output to import all findings into Defect Dojo, a vulnerability aggregation tool similar to ThreadFix.
The "old" pre-v7 json report contained from descriptive fields to inform the user of the vulnerbaility found, how to mitigate it and where to find more info and/or git commits.
Some of this information is missing in v7, rendering the information in the json report to be of less use.

v6 output for 1227

    "1227": {
      "findings": [
        {
          "version": "7.2.0",
          "paths": [
            "highcharts"
          ]
        }
      ],
      "id": 1227,
      "created": "2019-10-23T15:06:43.368Z",
      "updated": "2020-08-25T13:40:59.771Z",
      "deleted": null,
      "title": "Cross-Site Scripting",
      "found_by": {
        "link": "",
        "name": "François Lajeunesse-Robert",
        "email": ""
      },
      "reported_by": {
        "link": "",
        "name": "François Lajeunesse-Robert",
        "email": ""
      },
      "module_name": "highcharts",
      "cves": [],
      "vulnerable_versions": "<7.2.2 || >=8.0.0 <8.1.1",
      "patched_versions": ">=7.2.2 <8.0.0 || >=8.1.1",
      "overview": "Versions of `highcharts` prior to 7.2.2 or 8.1.1 are vulnerable to Cross-Site Scripting (XSS).  The package fails to sanitize `href` values and does not restrict URL schemes, allowing attackers to execute arbitrary JavaScript in a victim's browser if they click the link.",
      "recommendation": "Upgrade to version 7.2.2, 8.1.1 or later.",
      "references": "- [GitHub Issue](https://github.com/highcharts/highcharts/issues/13559)",
      "access": "public",
      "severity": "high",
      "cwe": "CWE-79",
      "metadata": {
        "module_type": "",
        "exploitability": 5,
        "affected_components": ""
      },
      "url": "https://npmjs.com/advisories/1227"
    },

npm7 output:

    "highcharts": {
      "name": "highcharts",
      "severity": "high",
      "via": [
        {
          "source": 1227,
          "name": "highcharts",
          "dependency": "highcharts",
          "title": "Cross-Site Scripting",
          "url": "https://npmjs.com/advisories/1227",
          "severity": "high",
          "range": "<7.2.2 || >=8.0.0 <8.1.1"
        }
      ],
      "effects": [],
      "range": "<7.2.2 || >=8.0.0 <8.1.1",
      "nodes": [
        "node_modules/highcharts"
      ],
      "fixAvailable": true
    },

Fields that are missing and which we are using / wanting back:

      "created": "2019-10-23T15:06:43.368Z",
      "updated": "2020-08-25T13:40:59.771Z",
      "cves": [],
      "patched_versions": ">=7.2.2 <8.0.0 || >=8.1.1",
      "overview": "Versions of `highcharts` prior to 7.2.2 or 8.1.1 are vulnerable to Cross-Site Scripting (XSS).  The package fails to sanitize `href` values and does not restrict URL schemes, allowing attackers to execute arbitrary JavaScript in a victim's browser if they click the link.",
      "recommendation": "Upgrade to version 7.2.2, 8.1.1 or later.",
      "references": "- [GitHub Issue](https://github.com/highcharts/highcharts/issues/13559)",
      "cwe": "CWE-79",
      "exploitability": 5,

Note: cve is empty for 1227 anyway, but for example 1518 has cve's, but they are absent in the npm7 output.

Steps to Reproduce

run npm audit --json with v6 and then v7 and observe the output difference

Expected Behavior

we are currently using the missing fields and I expected other tooling to use them too.
suggested behaviour is to reinstate the fields mentioned above.

@mrtnvh
Copy link

mrtnvh commented Nov 3, 2020

Hi @valentijnscholten ,

After some digging, I found that NPM exposes the https://registry.npmjs.org/-/npm/v1/security/advisories/{id} endpoint where ID translates to the [dependency-name].via.id value in the audit JSON.

Eg. Highcharts vulnerability

GET https://registry.npmjs.org/-/npm/v1/security/advisories/1227

{
  "id": 1227,
  "created": "2019-10-23T15:06:43.368",
  "updated": "2020-08-25T13:40:59.771",
  "deleted": null,
  "title": "Cross-Site Scripting",
  "found_by": { "link": "", "name": "François Lajeunesse-Robert", "email": "" },
  "reported_by": {
    "link": "",
    "name": "François Lajeunesse-Robert",
    "email": ""
  },
  "module_name": "highcharts",
  "cves": [],
  "vulnerable_versions": "<7.2.2 || >=8.0.0 <8.1.1",
  "patched_versions": ">=7.2.2 <8.0.0 || >=8.1.1",
  "overview": "Versions of `highcharts` prior to 7.2.2 or 8.1.1 are vulnerable to Cross-Site Scripting (XSS).  The package fails to sanitize `href` values and does not restrict URL schemes, allowing attackers to execute arbitrary JavaScript in a victim's browser if they click the link.",
  "recommendation": "Upgrade to version 7.2.2, 8.1.1 or later.",
  "references": "- [GitHub Issue](https://github.com/highcharts/highcharts/issues/13559)",
  "access": "public",
  "severity": "high",
  "cwe": "CWE-79",
  "metadata": {
    "module_type": "",
    "exploitability": 5,
    "affected_components": ""
  },
  "urls": {
    "detail": "/-/npm/v1/security/advisories/1227",
    "prev": "/-/npm/v1/security/advisories/1223",
    "next": "/-/npm/v1/security/advisories/1228"
  }
}

@valentijnscholten
Copy link
Author

I know that. But would like to see the fields included in npm audit output to avoid having to implement another follow up script to do that and having to handle errors, timeouts, ratelimiting, authentication, proxies, environments without internet, etc.

Is there a reason why the fields are not / cannot be included?

@valentijnscholten valentijnscholten changed the title [BUG] information missing in npm7 audit report information missing in npm7 audit report Oct 30, 2021
@valentijnscholten
Copy link
Author

I guess this relates to https://github.com/npm/npm-audit-report#break-from-version-1 which means I may have raised this issue in the wrong repo, but without diving into the source code I don't fully understand where the limitations are.

@isaacs @wraithgar Is there any chance (some of the) missing fields can be added back into the json report output? Without a CVE for example most vulnerability management programs will have a hard time using the json reports. Same for info about which version(s) fix/patch the vulnerability.

@isaacs
Copy link
Contributor

isaacs commented Nov 1, 2021

Adding fields to the JSON output seems reasonable enough. The data that we were reporting was kind of excessive, and ended up bloating the reports unnecessarily (for data that we always just threw away anyway). So, we scaled it back to what we knew we needed for npm's functionality.

Can you list the specific fields you actually need, so we can expand it only as necessary? You've mentioned CVE/CWE and patched_versions, which are both pretty small. Is there anything else that you're depending on specifically?

@isaacs
Copy link
Contributor

isaacs commented Nov 1, 2021

Ah, ok, this is a little bit more complicated, because it appears that the data we're reporting here is coming from the batch advisory endpoint, which is much faster, but reports much less data. So there would be either a server-side change, or a ton of additional requests to get the extra data (which we really don't want to do for performance reasons).

Still doable, but not as trivial as I thought a minute ago before actually looking at the code. 😬

@wraithgar
Copy link
Member

The bulk advisory endpoint is now returning cwe and cvss

$ curl -s -X POST -H 'Content-Type: application/json, application/octet-stream' https://registry.npmjs.org/-/npm/v1/security/advisories/bulk --data-binary '{"lodash":["1.3.1"]}'|json
{
  "lodash": [
    {
      "id": 1065345,
      "url": "https://github.com/advisories/GHSA-35jh-r3h4-6jhm",
      "title": "Command Injection in lodash",
      "severity": "high",
      "vulnerable_versions": "<4.17.21",
      "cwe": [
        "CWE-77"
      ],
      "cvss": {
        "score": 7.2,
        "vectorString": "CVSS:3.1/AV:N/AC:L/PR:H/UI:N/S:U/C:H/I:H/A:H"
      }
    },

I made a PR to include it in the metavuln report npm/metavuln-calculator#34

@valentijnscholten
Copy link
Author

@wraithgar Thanks for adding these fields. For a tool like Defect Dojo, or any other vulnerability management solution, it would still be "needed" to have the cve value included (and ideally the GHA id its own field). What would be the best way to get an answer from the npm registry team whether this will ever make it to the bulk endpoint output?

@devhops
Copy link

devhops commented Sep 27, 2022

I see this issue is closed, but the issue is still present. This is quite the blocker for me, is there any movement on adding the missing fields DefectDojo requires?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants