fix: improve limit option validation by Phillip9587 · Pull Request #698 · expressjs/body-parser

Phillip9587 · 2026-01-20T15:50:42Z

This PR fixes an issue where invalid limit options were silently accepted and effectively disabled request size limit enforcement.

Currently, when a user passed an invalid value (such as an unparseable string or NaN), bytes.parse() would return null, causing the request size limit validation to be skipped entirely. This happened without any error or warning, leaving users unaware that the configured limit was not being applied.

This PR makes the behavior explicit and predictable: null and undefined fall back to the default limit (100kb), valid values continue to work as before, and invalid values now throw a clear error instead of silently disabling the limit. This prevents misconfiguration and ensures request size limits are always enforced as expected.

This PR represents one of two possible approaches: either throwing an Error or logging a warning when an invalid option is provided. Throwing an Error seemed preferable, as it fails fast and makes configuration issues immediately visible.

At the moment there is also no explicit way to disable request size limit validation entirely. A possible follow-up for the next major version could be to change the behavior so that passing null explicitly disables the request limit check.

Copilot

Pull request overview

This PR fixes a security issue where invalid limit option values were silently accepted, causing request size limit enforcement to be disabled without warning. The fix makes validation explicit: null/undefined now fall back to the default 100kb limit, valid values continue to work as before, and invalid values throw a clear error immediately.

Changes:

Modified normalizeOptions in lib/utils.js to explicitly check for null/undefined limit values and throw errors for invalid values instead of silently accepting them
Added comprehensive test coverage for edge cases including undefined, null, 0, NaN, booleans, objects, and invalid strings

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
lib/utils.js	Updated limit validation logic to explicitly handle null/undefined with default value and throw TypeError for invalid limit values returned by bytes.parse()
test/utils.js	Added 8 new test cases covering undefined, null, zero, string without unit, invalid strings, NaN, booleans, and objects; removed old test that expected null for invalid input

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Chris de Almeida <ctcpip@users.noreply.github.com>

efekrskl

LGTM!

Phillip9587 · 2026-02-23T08:04:54Z

Hey @UlisesGascon @jonchurch, I would appreciate your feedback on this PR. Thanks!

Phillip9587 · 2026-02-23T08:05:32Z

Also #706 would be a good follow-up to this PR but is maybe a major.

jonchurch

Good bug fix, I think this should go out as a patch.

For the disabling limit in the future, limit: false seems reasonable and clear to me.

aagmab316

ok

…-111181 body-parser, qs: Crash on trailing bytes after a compressed body, ignored invalid limit, and qs stringify/comma-parse DoS (body-parser 1.20.5 -> 1.20.9, qs 6.15.2 -> 6.16.2) (#1207) Bumps `body-parser` from 1.20.5 to 1.20.9 and `qs` from 6.15.2 to 6.16.2 in `openam-ui/openam-ui-ria` (`package-lock.json` only), clearing Dependabot alert #389 (`qs`, GHSA-6pq8-jhm2-2vcw). The two move together: `body-parser` 1.20.5 pins `qs@~6.15.1`, and only 1.20.7 widened that to `~6.16.0`. This follows #1026, which took the module to `body-parser` 1.20.5 and `qs` 6.15.2 for CVE-2026-8723. Four more advisories cover the old versions; Dependabot auto-dismissed their alerts as development-scope — `body-parser` #274 (2026-07-21) and #381 (2026-10-27), `qs` #309 (2026-09-03) and #319 (2026-09-07) — so they are not open, but 1.20.5 and 6.15.2 are inside their ranges and this bump is what takes the module past them. `openam-ui-ria` is the only lock file in the repository that resolves either package. ## Advisories **CVE-2026-111181 / GHSA-27r3-jxmh-p828** (`body-parser`, fixed in 1.20.9) — Uncaught exception (CWE-248). When a request carries a `Content-Encoding` such as `gzip`, `read()` pipes it into a zlib decompression stream and, once the body has been read, leaves that stream piped with no `error` listener. A body of one valid compressed member followed by trailing bytes decodes and is answered normally, but the trailing bytes then reach the finished decoder, whose error is re-thrown as an uncaught exception and terminates the process. Every parser with the default `inflate: true` is affected, and since parsing runs before routing, no authentication or matching route is needed. 1.20.9 unpipes and destroys the decompression stream once the body is read, gives it a no-op `error` listener and drains the rest of the request; it also checks the charset before creating the stream, so an early 415 can no longer leave a decoder behind (expressjs/body-parser@1260c38). **CVE-2026-12590 / GHSA-v422-hmwv-36x6** (`body-parser`, fixed in 1.20.6) — Allocation of resources without limits (CWE-770). An invalid `limit` option — an unparseable string or `NaN` — made `bytes.parse()` return `null`, and the body size check was then silently skipped. 1.20.6 throws a `TypeError` for such a value when the parser is created; `null` and `undefined` still fall back to the 100 kB default (expressjs/body-parser#698). **CVE-2026-110816 / GHSA-6pq8-jhm2-2vcw** (`qs`, fixed in 6.16.1) — Uncaught exception; incorrect type conversion (CWE-248, CWE-704). `qs.stringify(obj, { arrayFormat: 'comma' })` coerces every array element to a string, through `Array.prototype.join` or, with `encodeValuesOnly`, through the value encoder. An element with an own non-callable `toString` — `{"a":[{"toString":"x"}]}` from a JSON body, or `a[0][toString]=x` parsed with `allowPrototypes: true` — makes it throw `TypeError: Cannot convert object to primitive value`. This is a different element type on the path that the CVE-2026-8723 fix (#1026) guarded only for `null` and `undefined`. 6.16.1 coerces the elements through `utils.toCoercible` before joining (ljharb/qs@a952e75). **CVE-2026-82417 / GHSA-4mjr-xmp4-gh2g** (`qs`, fixed in 6.16.0) — Uncaught exception (CWE-248, CWE-703). `utils.isBuffer()`, which `stringify` runs on every value, called `obj.constructor.isBuffer(obj)` without checking that it is a function, so a value such as `{"constructor":{"isBuffer":"x"}}` made `stringify` throw `TypeError`. `qs.parse` produces that shape itself from `x[constructor][isBuffer]=y` when `plainObjects: true` or `allowPrototypes: true` is set, so a `parse` → `stringify` round-trip of an untrusted query string is enough. 6.16.0 requires `typeof obj.constructor.isBuffer === 'function'` (ljharb/qs@e83d321). **CVE-2026-82562 / GHSA-x5fp-wj9c-mxmx** (`qs`, fixed in 6.16.0, affected since 6.14.2) — Allocation of resources without limits (CWE-770). With `comma: true`, `parseArrayValue()` enforced `arrayLimit` and `throwOnLimitExceeded` only for flat keys: the bracket form `a[]=1,2,3,4` was wrapped after splitting and its inner array was never checked, so a single parameter could build an array of any size. 6.16.0 counts the commas of every comma value, bracket key or not, before splitting it (ljharb/qs@8859c37). | | Severity | Affected (1.x / 6.x) | Fixed in | |---|---|---|---| | GHSA-27r3-jxmh-p828 (`body-parser`) | High — CVSS 3.1 7.5 (`AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H`) | < 1.20.9 | 1.20.9 | | GHSA-v422-hmwv-36x6 (`body-parser`) | Low — CVSS 3.1 3.7 (`AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:L`) | < 1.20.6 | 1.20.6 | | GHSA-6pq8-jhm2-2vcw (`qs`) | Medium — CVSS 3.1 5.3 (`AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:L`) | >= 6.7.0, <= 6.16.0 | 6.16.1 | | GHSA-4mjr-xmp4-gh2g (`qs`) | Medium — CVSS 3.1 5.3 (`AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:L`) | >= 2.2.5, < 6.16.0 | 6.16.0 | | GHSA-x5fp-wj9c-mxmx (`qs`) | Medium — CVSS 3.1 3.7 (`AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:L`), CVSS 4.0 6.3 | >= 6.14.2, <= 6.15.3 | 6.16.0 | All five are availability-only. The `body-parser` 2.x line received the same fixes in 2.4.0 and 2.3.0; it is not in this lock file. ## Impact on OpenAM None at runtime. Both packages are transitive development dependencies of `openam-ui-ria` (`dev: true`) and run only inside Node during the build; nothing from `node_modules` is copied into the module's zip (`target/compiled`) or into the XUI in the WAR. `body-parser` is required (`^1.19.0`) only by `karma` 6.4.3, the test runner. `qs` is required by `body-parser` (`~6.16.0`) and by `tiny-lr` 1.1.1 (`^6.4.0`), the LiveReload server of `grunt-contrib-watch` 1.1.0. No OpenAM source imports either. The Maven build does run one of them. The `npm-test` execution (`test` phase) runs `npm run test` → `grunt karma:build`, which starts a Karma server for the headless-Chrome run: - Karma binds `0.0.0.0` by default (`DEFAULT_LISTEN_ADDR`, overridable through `LISTEN_ADDR`), and `karma.conf.js` sets only `port: 9876`, so the server is reachable on every interface of the build host while the tests run. - Its runner middleware answers `/__run__` and `/run` and, once a browser is captured, parses the request with `require('body-parser').json()` — default options, so `inflate: true` and the 100 kB limit. That is exactly the GHSA-27r3 shape: one `POST /run` with `Content-Type: application/json`, `Content-Encoding: gzip` and bytes after the gzip member raises an uncaught exception, which Karma's own `uncaughtException` handler turns into a shutdown with exit code 1, failing the test phase. GHSA-v422 does not apply, since Karma passes no `limit`. - This gives an attacker on that network nothing new: the stopper middleware right after it shuts the server down on any request to `/stop` (`process.kill(process.pid, 'SIGINT')`), unauthenticated. Either way the worst outcome is an aborted test run; no code, data or artifact is affected. `qs` is never reached: - `body-parser` loads its parsers lazily and requires `qs` only for the `urlencoded` parser. Karma uses only `json()`, so on that path `qs` is not even loaded. - `tiny-lr` calls only `qs.parse(query)` with default options — no `comma`, `plainObjects` or `allowPrototypes` — and `grunt-contrib-watch` starts it only when a `livereload` option is set. The Gruntfile's `watch` task sets none, and it runs only under `grunt dev` (`npm start`), never in the Maven build. - Nothing in the tree calls `qs.stringify`, the sink of GHSA-6pq8 and GHSA-4mjr, and nothing passes `comma: true`, which GHSA-x5fp needs. Only GHSA-27r3 is reachable here, and only as a second way to end a test run that can already be stopped. The bump takes the build toolchain out of the affected ranges. ## Change `openam-ui/openam-ui-ria/package-lock.json` only; `package.json` is untouched. No code change. - `body-parser` 1.20.5 → 1.20.9 (`version`, `resolved`, `integrity`). Its `qs` range `~6.15.1` → `~6.16.0` is the only dependency change between the two. - `qs` 6.15.2 → 6.16.2. 6.16.x adds `es-define-property` (`^1.0.1`, already in the tree at 1.0.1) and raises `side-channel` to `^1.1.1`. The top-level `side-channel` 1.1.0 stays for `internal-slot` (`^1.1.0`), so npm nests `side-channel` 1.1.1 under `qs/node_modules`, together with `side-channel-list` 1.0.1, which 1.1.1 needs (`^1.0.1`) and the top-level 1.0.0 does not satisfy. Its other dependencies (`side-channel-map` 1.0.1, `side-channel-weakmap` 1.0.2, `object-inspect` 1.13.4, `es-errors` 1.3.0) resolve to the existing top-level copies. - `tiny-lr`'s `^6.4.0` is satisfied by 6.16.2, so it uses the same copy. 6.16.2 is the newest `qs` that `body-parser`'s `~6.16.0` allows (6.17.0 is out of that range), and 1.20.9 is the newest `body-parser` 1.x. Apart from the fixes above, v6.15.2...v6.16.2 adds a `depth` option to `stringify` and a number of parse and stringify bug fixes; `body-parser` 1.20.8 is a re-release of 1.20.7 that tested the new release process. 1.20.9 is published from GitHub Actions through npm trusted publishing, which is the new releaser Dependabot reports. Verified: - all four changed `integrity` hashes match the npm registry and the SHA-512 of the downloaded tarballs: `body-parser@1.20.9`, `qs@6.16.2`, `side-channel@1.1.1`, `side-channel-list@1.0.1` - the published `body-parser@1.20.9` carries the fixes: `lib/read.js` unpipes and destroys the decompression stream after the body is read, with an `error` listener, and checks the charset before creating it; `lib/types/json.js` rejects an invalid `limit`. Its registry entry was published by the `GitHub Actions` trusted publisher with an SLSA provenance attestation, and its `gitHead` is expressjs/body-parser@a64f140 (`1.20.9`) - the published `qs@6.16.2` carries the fixes: `utils.isBuffer` checks that `constructor.isBuffer` is a function, the comma branch of `stringify` joins through `utils.toCoercible`, and `parseArrayValue` counts the commas of bracket-key values too - `karma@6.4.3` requires `body-parser` only for `json()` in `lib/middleware/runner.js` and binds `0.0.0.0` by default; `tiny-lr@1.1.1` uses `qs` only for `qs.parse` in `lib/server.js`; `grunt-contrib-watch@1.1.0` starts `tiny-lr` only under a `livereload` option - the `openam-ui-js-sdk` and `openam-ui-api` lock files do not resolve `qs` or `body-parser` - the GitHub Advisory Database lists no advisory affecting `body-parser@1.20.9`, `qs@6.16.2`, `side-channel@1.1.1` or `side-channel-list@1.0.1` ## References - GHSA-27r3-jxmh-p828 - GHSA-v422-hmwv-36x6 - GHSA-6pq8-jhm2-2vcw - GHSA-4mjr-xmp4-gh2g - GHSA-x5fp-wj9c-mxmx - https://nvd.nist.gov/vuln/detail/CVE-2026-111181 - https://nvd.nist.gov/vuln/detail/CVE-2026-12590 - https://nvd.nist.gov/vuln/detail/CVE-2026-110816 - https://nvd.nist.gov/vuln/detail/CVE-2026-82417 - https://nvd.nist.gov/vuln/detail/CVE-2026-82562 - expressjs/body-parser@1260c38 - expressjs/body-parser#698 - ljharb/qs@a952e75 - ljharb/qs@e83d321 - ljharb/qs@8859c37 - https://github.com/expressjs/body-parser/releases/tag/1.20.9 - https://github.com/ljharb/qs/releases/tag/v6.16.1 - expressjs/body-parser@1.20.5...1.20.9 - ljharb/qs@v6.15.2...v6.16.2 Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

fix: improve limit option validation

a6e2ed1

Phillip9587 requested review from UlisesGascon, bjohansebas and jonchurch January 20, 2026 15:50

Phillip9587 added the bug label Jan 20, 2026

ctcpip requested changes Jan 20, 2026

View reviewed changes

Comment thread lib/utils.js Resolved

Phillip9587 requested a review from Copilot January 31, 2026 10:25

Copilot started reviewing on behalf of Phillip9587 January 31, 2026 10:26

View session

Copilot AI reviewed Jan 31, 2026

View reviewed changes

fix: use typeof for undefined check

a639447

Co-authored-by: Chris de Almeida <ctcpip@users.noreply.github.com>

Phillip9587 requested a review from ctcpip February 2, 2026 11:20

efekrskl approved these changes Feb 5, 2026

View reviewed changes

Phillip9587 mentioned this pull request Feb 23, 2026

fix: validate that limit option is non-negative #706

Open

5 tasks done

jonchurch approved these changes Mar 4, 2026

View reviewed changes

UlisesGascon added the semver-patch label Mar 4, 2026

UlisesGascon approved these changes Mar 4, 2026

View reviewed changes

UlisesGascon merged commit 2322e11 into expressjs:master Mar 4, 2026
16 checks passed

Phillip9587 deleted the limit-option-validation branch March 4, 2026 09:49

This was referenced Jun 10, 2026

2.2.3 #734

Closed

2.3.0 #735

Merged

Phillip9587 mentioned this pull request Jul 6, 2026

fix: improve limit option validation #741

Merged

This was referenced Jul 20, 2026

[VULN] Security Alert for body-parser srm-test-automation/test-auto-2#1729

Open

[VULN] Security Alert for body-parser srm-test-automation/test-auto-2#1730

Open

aagmab316 reviewed Jul 21, 2026

View reviewed changes

Mayvis mentioned this pull request Jul 22, 2026

deps: bump body-parser to ^2.3.0 to fix CVE-2026-12590 expressjs/express#7390

Merged

This was referenced Jul 22, 2026

[VULN] Security Alert for body-parser srm-test-automation/test-auto-2#2200

Open

[VULN] Security Alert for body-parser srm-test-automation/test-auto-2#2216

Open

srm-feature3-dev-test Bot mentioned this pull request Jul 22, 2026

[VULN] Security Alert for body-parser git-interface-test/test-app#51

Open

buildguy mentioned this pull request Sep 30, 2026

Bump brace-expansion from 1.1.11 to 1.1.21 webdetails/ccc#427

Open

buildguy mentioned this pull request Oct 17, 2026

Bump handlebars from 4.0.12 to 4.7.10 webdetails/ccc#428

Open

srm-local-dev-test Bot mentioned this pull request Oct 18, 2026

[VULN] Security Alert for body-parser san-advanced-test/test-auto-5#16

Open

buildguy mentioned this pull request Oct 19, 2026

Bump handlebars from 4.0.11 to 4.7.10 webdetails/cdf#1235

Open

This was referenced Oct 23, 2026

[VULN] Security Alert for body-parser san-advanced-test/test-auto-5#35

Open

[VULN] Security Alert for body-parser san-advanced-test/test-auto-5#56

Open

srm-local-dev-test Bot mentioned this pull request Oct 28, 2026

[VULN] Security Alert for body-parser san-advanced-test/test-auto-6#6

Open

github-actions Bot mentioned this pull request Oct 30, 2026

hello-world: GHSA-v422-hmwv-36x6 in pkg:npm/body-parser@1.20.3 Naguco/github-avr-poc#86

Open

Navigation Menu

Repository navigation

fix: improve limit option validation - #698

fix: improve limit option validation#698
UlisesGascon merged 2 commits into
expressjs:masterfrom
Phillip9587:limit-option-validation

Phillip9587 commented Jan 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

efekrskl left a comment

Uh oh!

Phillip9587 commented Feb 23, 2026

Uh oh!

Phillip9587 commented Feb 23, 2026

Uh oh!

jonchurch left a comment

Uh oh!

Uh oh!

aagmab316 left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

Phillip9587 commented Jan 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

efekrskl left a comment

Choose a reason for hiding this comment

Uh oh!

Phillip9587 commented Feb 23, 2026

Uh oh!

Phillip9587 commented Feb 23, 2026

Uh oh!

jonchurch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aagmab316 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants