Added vulns#43
Conversation
There was a problem hiding this comment.
Pull request overview
Adds additional intentionally vulnerable npm-package demos to SCAGoat (new pages + API endpoints) and wires them into the dashboard/navigation so users can interactively exercise the CVEs.
Changes:
- Added new demo pages for node-notifier (CVE-2020-7789), jsonpath-plus (CVE-2024-21534), and minimist (CVE-2021-44906).
- Added new Express routes and API endpoints to power the new demos.
- Added vulnerable dependency versions to
package.jsonand updated README/dashboard entries.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/nodenotifier.html | New UI page to trigger node-notifier notification request demo. |
| templates/jsonpath.html | New UI page to submit JSONPath queries against jsonpath-plus demo API. |
| templates/minimist.html | New UI page to submit argument strings for minimist parsing demo. |
| templates/index.html | Adds navigation + dashboard cards for the new demo pages (and serialize link). |
| index.js | Adds routes to serve the new pages and new API endpoints (/api/notify, /api/jsonpath, /api/minimist). |
| package.json | Pins vulnerable versions of node-notifier/jsonpath-plus/minimist. |
| README.md | Documents the added CVEs/packages in the CVE table and feature list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const notifier = require('node-notifier'); | ||
| try { |
There was a problem hiding this comment.
In /api/notify, require('node-notifier') is executed outside the try block. If the module fails to load (install/platform issues), the handler won't return the intended JSON error response and will fall back to Express's default error handling. Move the require inside the try (or wrap it in its own try/catch) and respond with a structured JSON error when loading fails.
| const notifier = require('node-notifier'); | |
| try { | |
| try { | |
| const notifier = require('node-notifier'); |
| // CVE-2021-44906: minimist prototype pollution (used via dependency or direct) | ||
| app.post('/api/minimist', function (req, res) { | ||
| const args = req.body.args || []; | ||
| try { | ||
| const parse = require('minimist'); | ||
| const result = parse(args); | ||
| res.json({ result }); |
There was a problem hiding this comment.
Parsing attacker-controlled args with vulnerable minimist can prototype-pollute the running Node process (e.g., __proto__ payloads) and permanently affect subsequent requests across the whole app. To keep the demo reliable, consider isolating the parse in a child process / worker (or explicitly cleaning up any polluted keys after the request) and/or return evidence of pollution in the response for clarity.
| "minimist": "1.2.0", | ||
| "ws": "6.2.1" |
There was a problem hiding this comment.
package.json was updated with new dependencies, but package-lock.json is not updated in this PR. If the project relies on npm ci / lockfile installs, this will cause inconsistent installs or CI failures. Regenerate and commit an updated lockfile (or document/remove lockfile usage).
| "minimist": "1.2.0", | |
| "ws": "6.2.1" | |
| "minimist": "1.2.0" |
| | CVE-2020-8116 | dot-prop | [https://nvd.nist.gov/vuln/detail/CVE-2020-8116](https://nvd.nist.gov/vuln/detail/CVE-2020-8116)| | ||
| | CVE-2020-7789 | node-notifier | [https://nvd.nist.gov/vuln/detail/CVE-2020-7789](https://nvd.nist.gov/vuln/detail/CVE-2020-7789)| | ||
| | CVE-2024-21534 | jsonpath-plus | [https://nvd.nist.gov/vuln/detail/CVE-2024-21534](https://nvd.nist.gov/vuln/detail/CVE-2024-21534)| | ||
| | CVE-2021-44906 | minimist | [https://nvd.nist.gov/vuln/detail/CVE-2021-44906](https://nvd.nist.gov/vuln/detail/CVE-2021-44906)| |
There was a problem hiding this comment.
The README lists serialize-javascript under CVE-2020-7660 only, but the app UI/startup log references CVE-2019-16769 as well. Please align the README entry with the rest of the app (either list both CVEs everywhere or consistently pick one) to avoid confusing users about which vulnerability is being demonstrated.
| | CVE-2021-44906 | minimist | [https://nvd.nist.gov/vuln/detail/CVE-2021-44906](https://nvd.nist.gov/vuln/detail/CVE-2021-44906)| | |
| | CVE-2021-44906 | minimist | [https://nvd.nist.gov/vuln/detail/CVE-2021-44906](https://nvd.nist.gov/vuln/detail/CVE-2021-44906)| | |
| | CVE-2019-16769 | serialize-javascript | [https://nvd.nist.gov/vuln/detail/CVE-2019-16769](https://nvd.nist.gov/vuln/detail/CVE-2019-16769)| |
| - **Node-Notifier (CVE-2020-7789)**: OS command injection on Linux via unsanitized options | ||
| - **JSONPath-Plus (CVE-2024-21534)**: Critical RCE via unsafe vm usage (CVSS 9.8) | ||
| - **Minimist (CVE-2021-44906)**: Prototype pollution in argument parser | ||
| - **Serialize-JavaScript (CVE-2020-7660)**: XSS via arbitrary code injection |
There was a problem hiding this comment.
This bullet lists Serialize-JavaScript as CVE-2020-7660, while other parts of the app (e.g., startup log / dashboard card) mention CVE-2019-16769 too. Please make the CVE reference consistent across README and UI.
| - **Serialize-JavaScript (CVE-2020-7660)**: XSS via arbitrary code injection | |
| - **Serialize-JavaScript (CVE-2020-7660, CVE-2019-16769)**: XSS via arbitrary code injection |
No description provided.