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

[BUG](arborist) Wrong vulnerabilities number calculation algorithm in second version of audit report #4272

Open
ar1em opened this issue Oct 21, 2021 · 0 comments
Labels
Needs Triage needs review for next steps ws:arborist Related to the arborist workspace

Comments

@ar1em
Copy link

ar1em commented Oct 21, 2021

What / Why

Section "metadata -> vulnerabilities" of second version of audit report contains number of vulnerable packages instead of vulnerabilities number.

Actually "vulnerabilities" section contains vulnerable packages instead of vulnerabilities.

There is no longer a "paths" field that showed what dependencies depend on the vulnerable package. Previous version of npm audit (v6.14.15) used "paths" field to calculate vulnerabilities number. npm audit (^7.0.0) has "nodes" field in the report, but it is not the same.

Steps to Reproduce and Current Behavior

  1. create package.json
{
  "name": "npm-audit-bug-example",
  "version": "1.2.12",
  "description": "npm-audit-bug-example",
  "license": "MIT",
  "dependencies": {
    "angular2-highcharts": "^0.5.5",
    "highcharts": "^8.2.0"
  }
}
  1. install npm install -g npm@8.0.0
  2. run npm install
  3. run npm audit --json
    In the example below, angular2-highcharts depends on vulnerable highcharts. Installed version of highcharts contains 3 vulnerability: 1002707, 1004028, 1004388. The number of high vulnerabilities can not be equal - 2. The counting algorithm may differ, in my opinion it was implemented correctly in the previous version npm (v6.14.15).
{
  "auditReportVersion": 2,
  "vulnerabilities": {
    "angular2-highcharts": {
      "name": "angular2-highcharts",
      "severity": "high",
      "isDirect": true,
      "via": [
        "highcharts"
      ],
      "effects": [],
      "range": ">=0.0.2",
      "nodes": [
        "node_modules/angular2-highcharts"
      ],
      "fixAvailable": {
        "name": "angular2-highcharts",
        "version": "0.0.1",
        "isSemVerMajor": true
      }
    },
    "highcharts": {
      "name": "highcharts",
      "severity": "high",
      "isDirect": true,
      "via": [
        {
          "source": 1002707,
          "name": "highcharts",
          "dependency": "highcharts",
          "title": "Options structure open to XSS if passed unfiltered",
          "url": "https://github.com/advisories/GHSA-8j65-4pcq-xq95",
          "severity": "high",
          "range": "<9.0.0"
        },
        {
          "source": 1004028,
          "name": "highcharts",
          "dependency": "highcharts",
          "title": "Cross-Site Scripting in highcharts",
          "url": "https://github.com/advisories/GHSA-gr4j-r575-g665",
          "severity": "high",
          "range": "<7.2.2"
        },
        {
          "source": 1004388,
          "name": "highcharts",
          "dependency": "highcharts",
          "title": "Regular Expression Denial of Service in highcharts",
          "url": "https://github.com/advisories/GHSA-xmc8-cjfr-phx3",
          "severity": "high",
          "range": "<6.1.0"
        }
      ],
      "effects": [
        "angular2-highcharts"
      ],
      "range": "<=8.2.2",
      "nodes": [
        "node_modules/angular2-highcharts/node_modules/highcharts",
        "node_modules/highcharts"
      ],
      "fixAvailable": {
        "name": "highcharts",
        "version": "9.2.2",
        "isSemVerMajor": true
      }
    }
  },
  "metadata": {
    "vulnerabilities": {
      "info": 0,
      "low": 0,
      "moderate": 0,
      "high": 2,
      "critical": 0,
      "total": 2
    },
    "dependencies": {
      "prod": 6,
      "dev": 0,
      "optional": 0,
      "peer": 0,
      "peerOptional": 0,
      "total": 5
    }
  }
}

Expected Behavior

  1. angular2-highcharts should not be shown as separate vulnerability
  2. "vulnerabilities -> highcharts" should have "paths" field that shows dependencies depend on the vulnerable package
  3. "metadata -> vulnerabilities" should be calculated base on "paths" fields.
    This is how it worked in the previous version of npm (before v7), and it was excellent. Example of audit result using previous version (v6.14.15).
...
"advisories": {
    "1002707": {
      "findings": [
        {
          "version": "5.0.15",
          "paths": [
            "angular2-highcharts>highcharts"
          ]
        },
        {
          "version": "8.2.2",
          "paths": [
            "highcharts"
          ]
        }
      ],
      "metadata": null,
      "vulnerable_versions": "<9.0.0",
      "module_name": "highcharts",
      "severity": "high",
      "github_advisory_id": "GHSA-8j65-4pcq-xq95",
      "cves": [
        "CVE-2021-29489"
      ],
      "access": "public",
      "patched_versions": ">=9.0.0",
      "updated": "2021-05-06T15:44:24.000Z",
      "recommendation": "Upgrade to version 9.0.0 or later",
      "cwe": "CWE-79",
      "found_by": null,
      "deleted": null,
      "id": 1002707,
      "references": "- https://github.com/highcharts/highcharts/security/advisories/GHSA-8j65-4pcq-xq95\n- https://nvd.nist.gov/vuln/detail/CVE-2021-29489\n- https://github.com/advisories/GHSA-8j65-4pcq-xq95",
      "created": "2021-10-07T07:31:50.547Z",
      "reported_by": null,
      "title": "Options structure open to XSS if passed unfiltered",
      "npm_advisory_id": null,
      "overview": "### Impact\nIn Highcharts versions 8 and earlier, the chart options structure was not systematically filtered for XSS vectors. The potential impact was that content from untrusted sources could execute code in the end user's browser. Especially when using the `useHTML` flag, HTML string options would be inserted unfiltered directly into the DOM. When `useHTML` was false, malicious code could be inserted by using various character replacement tricks or malformed HTML.\n\nIf your chart configuration comes from a trusted source like a static setup or pre-filtered HTML (or no markup at all in the configuration), you are not impacted.\n\n### Patches\nIn version 9, the whole rendering layer was refactored to use an DOMParser, an AST and tag and HTML allow-listing to make sure only safe content entered the DOM. In addition, prototype pollution was stopped.\n\n### Workarounds\nImplementers who are not able to upgrade may apply [DOMPurify](https://github.com/cure53/DOMPurify) recursively [to the options structure](https://jsfiddle.net/highcharts/zd3wcm5L/) to filter out malicious markup.\n\n### References\n* Details on the improved [Highcharts security](https://www.highcharts.com/docs/chart-concepts/security)\n* [The AST and TextBuilder refactoring](https://github.com/highcharts/highcharts/pull/14913)\n* [The fix for prototype pollution](https://github.com/highcharts/highcharts/pull/14884)\n\n### For more information\nIf you have any questions or comments about this advisory:\n* Visit our [support page](https://www.highcharts.com/blog/support/)\n* For more Email us at [security@highcharts.com](mailto:security@highcharts.com)\n",
      "url": "https://github.com/advisories/GHSA-8j65-4pcq-xq95"
    },
    "1004028": {
      "findings": [
        {
          "version": "5.0.15",
          "paths": [
            "angular2-highcharts>highcharts"
          ]
        }
      ],
      "metadata": null,
      "vulnerable_versions": "<7.2.2",
      "module_name": "highcharts",
      "severity": "high",
      "github_advisory_id": "GHSA-gr4j-r575-g665",
      "cves": [],
      "access": "public",
      "patched_versions": ">=7.2.2",
      "updated": "2020-08-25T14:01:39.000Z",
      "recommendation": "Upgrade to version 7.2.2 or later",
      "cwe": "CWE-79",
      "found_by": null,
      "deleted": null,
      "id": 1004028,
      "references": "- https://github.com/highcharts/highcharts/issues/13559\n- https://github.com/advisories/GHSA-gr4j-r575-g665",
      "created": "2021-10-07T07:31:50.673Z",
      "reported_by": null,
      "title": "Cross-Site Scripting in highcharts",
      "npm_advisory_id": null,
      "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.",
      "url": "https://github.com/advisories/GHSA-gr4j-r575-g665"
    },
    "1004388": {
      "findings": [
        {
          "version": "5.0.15",
          "paths": [
            "angular2-highcharts>highcharts"
          ]
        }
      ],
      "metadata": null,
      "vulnerable_versions": "<6.1.0",
      "module_name": "highcharts",
      "severity": "high",
      "github_advisory_id": "GHSA-xmc8-cjfr-phx3",
      "cves": [
        "CVE-2018-20801"
      ],
      "access": "public",
      "patched_versions": ">=6.1.0",
      "updated": "2019-03-18T15:59:21.000Z",
      "recommendation": "Upgrade to version 6.1.0 or later",
      "cwe": "CWE-185",
      "found_by": null,
      "deleted": null,
      "id": 1004388,
      "references": "- https://nvd.nist.gov/vuln/detail/CVE-2018-20801\n- https://github.com/advisories/GHSA-xmc8-cjfr-phx3",
      "created": "2021-10-07T07:31:50.704Z",
      "reported_by": null,
      "title": "Regular Expression Denial of Service in highcharts",
      "npm_advisory_id": null,
      "overview": "Versions of `highcharts` prior to 6.1.0 are vulnerable to Regular Expression Denial of Service (ReDoS). Untrusted input may cause catastrophic backtracking while matching regular expressions. This can cause the application to be unresponsive leading to Denial of Service.\n\n\n## Recommendation\n\nUpgrade to version 6.1.0 or higher.",
      "url": "https://github.com/advisories/GHSA-xmc8-cjfr-phx3"
    }
  },
  "muted": [],
  "metadata": {
    "vulnerabilities": {
      "info": 0,
      "low": 0,
      "moderate": 0,
      "high": 4,
      "critical": 0
    },
    "dependencies": 5,
    "devDependencies": 0,
    "optionalDependencies": 0,
    "totalDependencies": 5
  }
...

Versions

npm -v
8.0.0

@fritzy fritzy transferred this issue from npm/arborist Jan 20, 2022
@fritzy fritzy added Needs Triage needs review for next steps ws:arborist Related to the arborist workspace labels Jan 20, 2022
@fritzy fritzy changed the title [BUG] Wrong vulnerabilities number calculation algorithm in second version of audit report [BUG](arborist) Wrong vulnerabilities number calculation algorithm in second version of audit report Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Triage needs review for next steps ws:arborist Related to the arborist workspace
Projects
None yet
Development

No branches or pull requests

2 participants