[Fix] stringify: skip null/undefined filter-array entries instead of crashing in encoder - #551
Merged
Merged
Conversation
ljharb
requested changes
Apr 15, 2026
ljharb
left a comment
Owner
There was a problem hiding this comment.
This would need regression tests.
ljharb
marked this pull request as draft
April 15, 2026 23:10
Author
|
Regression test added, thanks for reminding me! |
gregkh
marked this pull request as ready for review
April 19, 2026 09:18
stringify: skip null/undefined filter-array entries instead o…stringify: skip null/undefined filter-array entries instead of crashing in encoder
…f crashing in `encoder` When `filter` is an array, its elements become `objKeys` directly and the top-level loop passes each entry as the recursive `prefix` argument without coercion. A `null` or `undefined` entry reached `utils.encode` as the `str` argument and `str.length` threw `TypeError`. The crash requires two conditions to align: the filter array must contain `null`/`undefined`, and the object being stringified must have a literal `'null'`/`'undefined'` property (otherwise `obj[key]` is `undefined` and the typeof-undefined early-return at stringify.js:137 fires before the encoder is called) Both are reachable from an attacker controlled input in applications that stringify user-submitted JSON with user-selected fields. Fix this up by skipping `null`/`undefined` filter entries at the top-level loop. This somewhat matches `JSON.stringify` replacer-array semantics, which silently ignore non-string/non-number entries. `Object.keys` never yields `null`/`undefined`, so the guard only affects user-supplied filter arrays. The inner recursive loop already coerces `key` via `String()` when building `keyPrefix`, so it never passes a raw `null` to the encoder.
ljharb
approved these changes
Apr 27, 2026
ljharb
left a comment
Owner
There was a problem hiding this comment.
Although I'd prefer you didn't use LLMs for open source contributions, this seems fine, thanks.
ljharb
force-pushed
the
stringify-skip-null
branch
from
April 27, 2026 05:59
89913af to
0c180a4
Compare
Author
Sorry about that, a LLM found the problem, and helped with a portion of this. If there is a better way to mark this as such, I'll be glad to do so. Thanks for taking the change, much appreciated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
filteris an array, its elements becomeobjKeysdirectly and the top-level loop passes each entry as the recursiveprefixargument without coercion. Anullorundefinedentry reachedutils.encodeas thestrargument andstr.lengththrewTypeError.The crash requires two conditions to align: the filter array must contain
null/undefined, and the object being stringified must have a literal'null'/'undefined'property (otherwiseobj[key]isundefinedand the typeof-undefined early-return at stringify.js:137 fires before the encoder is called). Both are reachable from an attacker controlled input in applications that stringify user-submitted JSON with user-selected fields.Fix this up by skipping
null/undefinedfilter entries at the top-level loop. This matchesJSON.stringifyreplacer-array semantics, which silently ignore non-string/non-number entries.Object.keysnever yieldsnull/undefined, so the guard only affects user-supplied filter arrays. The inner recursive loop already coerceskeyviaString()when buildingkeyPrefix, so it never passes a rawnullto the encoder.