Problem/Motivation

The braces NPM package, a dependency used in Drupal's core JavaScript tooling, has a known vulnerability in versions earlier than 3.0.3. This vulnerability allows for uncontrolled resource consumption, potentially leading to memory exhaustion and a crash. Specifically, when parsing input with "imbalanced braces," the package may enter an infinite loop, allocating heap memory without releasing it.

This issue was identified by a Dependabot alert, which could not automatically update the package due to the absence of a package.json in the web/core/ directory. The vulnerability needs to be addressed to ensure the security and stability of Drupal's core dependencies.

Steps to reproduce

  1. Use a version of Drupal that depends on braces version 3.0.2 or earlier.
  2. Parse input with "imbalanced braces" using the braces package.
  3. Observe that the process consumes increasing amounts of memory and eventually crashes due to memory exhaustion.

Proposed resolution

  • Update the braces package in Drupal's dependencies to version 3.0.3 by modifying the package.json and yarn.lock files.
  • Ensure that all transitive dependencies are also using braces version 3.0.3.
  • Verify that the issue is resolved by testing the behavior with inputs that previously caused memory exhaustion.
  • Submit a patch to apply these changes to the Drupal core repository.

Remaining tasks

  • Apply the braces update to the relevant branches (10.3.x).
  • Verify that the update does not introduce any regressions or new issues.
  • Submit the patch for review.
  • Communicate with the maintainers to ensure the patch is merged into the appropriate Drupal core versions.
CommentFileSizeAuthor
#2 core-fix-braces-vulnerability-3467599-1.patch1.87 KBhossein.daliran

Issue fork drupal-3467599

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

hossein.daliran created an issue. See original summary.

hossein.daliran’s picture

hossein.daliran commented
StatusFileSize
new core-fix-braces-vulnerability-3467599-1.patch1.87 KB

Patch for 10.2.x

cilefen’s picture

cilefen commented
Version: 10.3.x-dev » 11.x-dev
Category: Feature request » Task
Status: Active » Needs work
Issue tags: +Needs merge request
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: Update braces NPM package to version 3.0.3 to prevent uncontrolled resource consumption vulnerability » Update braces and ws NPM packages

yarn audit also reports problems with ws, let's fix that at the same time.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Note also that this only affects development dependencies and can't be exploited at runtime, the Drupal Security Team will not be issuing a security release or advisory about this change.

Spokje changed the visibility of the branch 3467599-update-braces-npm to hidden.

Spokje opened merge request !9187

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje

Let's see...

spokje’s picture

spokje
Primary language Dutch
commented

Oh, the joy of supporting multiple versions of core...
This looks like this is going to be a different MR for at least 11.x/10.x and perhaps even one for every minor version.

Lets start with 11.x.

Firstly in yarn v4-land apparently yarn audit now became yarn npm audit -R.

On 11.x that gives us:

$ yarn npm audit -R
├─ @humanwhocodes/config-array
│  ├─ ID: @humanwhocodes/config-array (deprecation)
│  ├─ Issue: Use @eslint/config-array instead
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 0.11.14
│  │
│  ├─ Tree Versions
│  │  └─ 0.11.14
│  │
│  └─ Dependents
│     └─ eslint@npm:8.57.0
│
├─ @humanwhocodes/object-schema
│  ├─ ID: @humanwhocodes/object-schema (deprecation)
│  ├─ Issue: Use @eslint/object-schema instead
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 2.0.3
│  │
│  ├─ Tree Versions
│  │  └─ 2.0.3
│  │
│  └─ Dependents
│     └─ @humanwhocodes/config-array@npm:0.11.14
│
├─ braces
│  ├─ ID: 1098094
│  ├─ Issue: Uncontrolled resource consumption in braces
│  ├─ URL: https://github.com/advisories/GHSA-grv7-fg5c-xmjg
│  ├─ Severity: high
│  ├─ Vulnerable Versions: <3.0.3
│  │
│  ├─ Tree Versions
│  │  └─ 3.0.2
│  │
│  └─ Dependents
│     └─ chokidar@npm:3.6.0
│
├─ glob
│  ├─ ID: glob (deprecation)
│  ├─ Issue: Glob versions prior to v9 are no longer supported
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 8.1.0
│  │
│  ├─ Tree Versions
│  │  └─ 8.1.0
│  │
│  └─ Dependents
│     └─ mocha@npm:10.3.0
│
├─ inflight
│  ├─ ID: inflight (deprecation)
│  ├─ Issue: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 1.0.6
│  │
│  ├─ Tree Versions
│  │  └─ 1.0.6
│  │
│  └─ Dependents
│     └─ glob@npm:7.2.3
│
├─ rimraf
│  ├─ ID: rimraf (deprecation)
│  ├─ Issue: Rimraf versions prior to v4 are no longer supported
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 3.0.2
│  │
│  ├─ Tree Versions
│  │  └─ 3.0.2
│  │
│  └─ Dependents
│     └─ flat-cache@npm:3.2.0
│
└─ tar
   ├─ ID: 1097493
   ├─ Issue: Denial of service while parsing a tar file due to lack of folders count validation
   ├─ URL: https://github.com/advisories/GHSA-f5x3-32g6-xq36
   ├─ Severity: moderate
   ├─ Vulnerable Versions: <6.2.1
   │
   ├─ Tree Versions
   │  └─ 6.2.0
   │
   └─ Dependents
      └─ node-gyp@npm:10.0.1

If we snip the deprecations that leaves us with:

├─ braces
│  ├─ ID: 1098094
│  ├─ Issue: Uncontrolled resource consumption in braces
│  ├─ URL: https://github.com/advisories/GHSA-grv7-fg5c-xmjg
│  ├─ Severity: high
│  ├─ Vulnerable Versions: <3.0.3
│  │
│  ├─ Tree Versions
│  │  └─ 3.0.2
│  │
│  └─ Dependents
│     └─ chokidar@npm:3.6.0
│
└─ tar
   ├─ ID: 1097493
   ├─ Issue: Denial of service while parsing a tar file due to lack of folders count validation
   ├─ URL: https://github.com/advisories/GHSA-f5x3-32g6-xq36
   ├─ Severity: moderate
   ├─ Vulnerable Versions: <6.2.1
   │
   ├─ Tree Versions
   │  └─ 6.2.0
   │
   └─ Dependents
      └─ node-gyp@npm:10.0.1
spokje’s picture

spokje
Primary language Dutch
commented
Title: Update braces and ws NPM packages » Update NPM packages flagged by yarn audit
spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs merge request

So the 11.x-branch didn't break tests.

Let the backport-games begin!

spokje’s picture

spokje
Primary language Dutch
commented
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

if I understand the thread a piece of the change should be reverted.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Needs review

Thanks @smustgrave, wasn't pinged on the comment on the MR.

Replied inline, back to NR to await the answer of @longwave.

Spokje closed merge request !9187

Spokje opened merge request !9243

spokje’s picture

spokje
Primary language Dutch
commented

Opened a new branch using the yarn up -R <name> method, not using resolutions.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Reviewed the MR with just the lock file and seems straight forward. Didn't cause any issues locally updating.

nod_ closed merge request !9243

  • nod_ committed 46ee7302 on 11.x 
    Issue #3467599 by Spokje, hossein.daliran, longwave, smustgrave: Update...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 11.0.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

11.x in, next branch is 11.0.x :)

Committed 46ee730 and pushed to 11.x. Thanks!

spokje’s picture

spokje
Primary language Dutch
commented

11.0.x:

$ yarn npm audit -R
├─ @humanwhocodes/config-array
│  ├─ ID: @humanwhocodes/config-array (deprecation)
│  ├─ Issue: Use @eslint/config-array instead
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 0.11.14
│  │
│  ├─ Tree Versions
│  │  └─ 0.11.14
│  │
│  └─ Dependents
│     └─ eslint@npm:8.57.0
│
├─ @humanwhocodes/object-schema
│  ├─ ID: @humanwhocodes/object-schema (deprecation)
│  ├─ Issue: Use @eslint/object-schema instead
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 2.0.3
│  │
│  ├─ Tree Versions
│  │  └─ 2.0.3
│  │
│  └─ Dependents
│     └─ @humanwhocodes/config-array@npm:0.11.14
│
├─ braces
│  ├─ ID: 1098094
│  ├─ Issue: Uncontrolled resource consumption in braces
│  ├─ URL: https://github.com/advisories/GHSA-grv7-fg5c-xmjg
│  ├─ Severity: high
│  ├─ Vulnerable Versions: <3.0.3
│  │
│  ├─ Tree Versions
│  │  └─ 3.0.2
│  │
│  └─ Dependents
│     └─ chokidar@npm:3.6.0
│
├─ ejs
│  ├─ ID: 1098366
│  ├─ Issue: ejs lacks certain pollution protection
│  ├─ URL: https://github.com/advisories/GHSA-ghr5-ch3p-vcr6
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: <3.1.10
│  │
│  ├─ Tree Versions
│  │  └─ 3.1.8
│  │
│  └─ Dependents
│     └─ nightwatch@npm:2.4.2 [87c89]
│
├─ glob
│  ├─ ID: glob (deprecation)
│  ├─ Issue: Glob versions prior to v9 are no longer supported
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 7.2.0
│  │
│  ├─ Tree Versions
│  │  └─ 7.2.0
│  │
│  └─ Dependents
│     └─ mocha@npm:9.2.2
│
├─ inflight
│  ├─ ID: inflight (deprecation)
│  ├─ Issue: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 1.0.6
│  │
│  ├─ Tree Versions
│  │  └─ 1.0.6
│  │
│  └─ Dependents
│     └─ glob@npm:7.2.3
│
├─ loupe
│  ├─ ID: loupe (deprecation)
│  ├─ Issue: Please upgrade to 2.3.7 which fixes GHSA-4q6p-r6v2-jvc5
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 2.3.4
│  │
│  ├─ Tree Versions
│  │  └─ 2.3.4
│  │
│  └─ Dependents
│     └─ @nightwatch/chai@npm:5.0.2
│
├─ micromatch
│  ├─ ID: 1098615
│  ├─ Issue: Regular Expression Denial of Service (ReDoS) in micromatch
│  ├─ URL: https://github.com/advisories/GHSA-952p-6rrq-rcjv
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: <=4.0.7
│  │
│  ├─ Tree Versions
│  │  ├─ 4.0.5
│  │  └─ 4.0.7
│  │
│  └─ Dependents
│     ├─ cspell-glob@npm:8.13.0
│     └─ stylelint@npm:15.11.0
│
├─ rimraf
│  ├─ ID: rimraf (deprecation)
│  ├─ Issue: Rimraf versions prior to v4 are no longer supported
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: 3.0.2
│  │
│  ├─ Tree Versions
│  │  └─ 3.0.2
│  │
│  └─ Dependents
│     └─ flat-cache@npm:3.2.0
│
├─ tar
│  ├─ ID: 1097493
│  ├─ Issue: Denial of service while parsing a tar file due to lack of folders count validation
│  ├─ URL: https://github.com/advisories/GHSA-f5x3-32g6-xq36
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: <6.2.1
│  │
│  ├─ Tree Versions
│  │  └─ 6.2.0
│  │
│  └─ Dependents
│     └─ node-gyp@npm:10.0.1
│
└─ ws
   ├─ ID: 1098392
   ├─ Issue: ws affected by a DoS when handling a request with many HTTP headers
   ├─ URL: https://github.com/advisories/GHSA-3h5v-q93c-6h6q
   ├─ Severity: high
   ├─ Vulnerable Versions: >=8.0.0 <8.17.1
   │
   ├─ Tree Versions
   │  └─ 8.15.1
   │
   └─ Dependents
      └─ selenium-webdriver@npm:4.3.1

That means we want to update:

$ yarn npm audit -R
├─ braces
│  ├─ ID: 1098094
│  ├─ Issue: Uncontrolled resource consumption in braces
│  ├─ URL: https://github.com/advisories/GHSA-grv7-fg5c-xmjg
│  ├─ Severity: high
│  ├─ Vulnerable Versions: <3.0.3
│  │
│  ├─ Tree Versions
│  │  └─ 3.0.2
│  │
│  └─ Dependents
│     └─ chokidar@npm:3.6.0
│
├─ ejs
│  ├─ ID: 1098366
│  ├─ Issue: ejs lacks certain pollution protection
│  ├─ URL: https://github.com/advisories/GHSA-ghr5-ch3p-vcr6
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: <3.1.10
│  │
│  ├─ Tree Versions
│  │  └─ 3.1.8
│  │
│  └─ Dependents
│     └─ nightwatch@npm:2.4.2 [87c89]
│
├─ micromatch
│  ├─ ID: 1098615
│  ├─ Issue: Regular Expression Denial of Service (ReDoS) in micromatch
│  ├─ URL: https://github.com/advisories/GHSA-952p-6rrq-rcjv
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: <=4.0.7
│  │
│  ├─ Tree Versions
│  │  ├─ 4.0.5
│  │  └─ 4.0.7
│  │
│  └─ Dependents
│     ├─ cspell-glob@npm:8.13.0
│     └─ stylelint@npm:15.11.0
│
├─ tar
│  ├─ ID: 1097493
│  ├─ Issue: Denial of service while parsing a tar file due to lack of folders count validation
│  ├─ URL: https://github.com/advisories/GHSA-f5x3-32g6-xq36
│  ├─ Severity: moderate
│  ├─ Vulnerable Versions: <6.2.1
│  │
│  ├─ Tree Versions
│  │  └─ 6.2.0
│  │
│  └─ Dependents
│     └─ node-gyp@npm:10.0.1
│
└─ ws
   ├─ ID: 1098392
   ├─ Issue: ws affected by a DoS when handling a request with many HTTP headers
   ├─ URL: https://github.com/advisories/GHSA-3h5v-q93c-6h6q
   ├─ Severity: high
   ├─ Vulnerable Versions: >=8.0.0 <8.17.1
   │
   ├─ Tree Versions
   │  └─ 8.15.1
   │
   └─ Dependents
      └─ selenium-webdriver@npm:4.3.1

Spokje opened merge request !9305

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje
spokje’s picture

spokje
Primary language Dutch
commented
Status: Patch (to be ported) » Needs review

Things got more involved with this one, after a $ yarn up -R braces ejs micromatch tar ws it turned out braces, ejs and micromatch we're not updated.

Next up, I tried removing the entry for each of them in yarn.lock and doing a yarn install.
That worked for micromatch, however that has a CVE stating we need to go 3.0.8 or higher (See https://github.com/advisories/GHSA-952p-6rrq-rcjv). However the latest release is 3.0.7 (See https://www.npmjs.com/package/micromatch).
So this one still gets flagged by yarn audit -R, but as far as I can tell there's a mistake made in the CVE.

Finally: braces, ejs could only be bumped by using resolutions. I know this is not optimal, but at least this will only happen in version branches and not in main/11.x

Also they all have resolutions already, due to having an ancient nightwatch version.

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

The reasoning in #25 I believe makes sense (js is not my strong area). But 11.0.x tests didn't break and updates for 11.x seem accepted.

nod_ closed merge request !9305

  • nod_ committed cf5738bc on 11.0.x 
    Issue #3467599 by Spokje, hossein.daliran, smustgrave, longwave, nod_:...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Fixed

branches below have a different yarn version/lock file format so let's to that in a different issue if necessary and leave this one here.

Committed cf5738b and pushed to 11.0.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.