Fuzzer issues - #1121
Merged
Merged
Conversation
VadimZhestikov
previously approved these changes
Sep 4, 2026
VadimZhestikov
left a comment
VadimZhestikov
left a comment
Contributor
There was a problem hiding this comment.
Few non-blocking notes:
- Commit hygiene: each commit names the introducing commit + version and the issues it closes — excellent for backport triage. Several are old (0.3.2/0.3.4/0.3.8), so backporting to maintained branches is worth considering.
- Minor: the git apply -R of commit 5 flagged a trailing-blank-line whitespace warning (commit 1 also removed a stray trailing blank line in external/njs_regex.c) — cosmetic, no impact.
- These are memory-safety bugs (type confusion, UAF, OOB r/w) reachable from script; under njs's trusted-script model they're most exploitable where a benign script feeds untrusted data into regex/replace/typed-array/defineProperty paths. Worth flagging for security-release handling given the count and severity.
github-project-automation
Bot
moved this from New
to In Review
in NGINX OSS Unified Workspace
Previously, RegExp.prototype[Symbol.replace]() and Symbol.split() used non-NULL exotic slots as proof that an exec() result was an internally allocated match array. A custom exec() could return any exotic object, which was then cast to an array and freed, corrupting unrelated VM state. The fix is to record whether RegExpBuiltinExec produced each result and to release only fresh builtin results that could not escape through user code. The replace issue was introduced in f781317 (0.8.2), and the split issue was introduced in aa697e3 (0.8.2). This closes nginx#1090, nginx#1091, nginx#1092, nginx#1093, nginx#1096, and nginx#1097 issues on GitHub.
Previously, Object.values() and Object.entries() erased the concrete type of an exotic receiver, while indexed property fast paths accepted any object with the fast_array bit. Typed arrays could consequently be interpreted as njs_array_t objects, causing invalid memory reads and writes. The fix is to preserve the receiver type during enumeration and to use njs_is_fast_array() for both indexed get and set fast paths. The issue was introduced in 6907216 (0.8.6); the overbroad fast path dates to 912ab38 (0.3.8). This closes nginx#1098 issue on GitHub.
Previously, the fast concat path represented a missing source property with njs_value_invalid and still created the corresponding destination property. For a slow sparse destination this materialized an internal hole sentinel as a normal property, which later property access could interpret as an invalid accessor. The fix is to advance the destination index without creating a property when the source lookup reports a hole. The issue was introduced in 232fb59 (0.7.2). This closes nginx#1095 issue on GitHub.
Previously, njs_object_prop_define() retained a pointer to a target hash property while descriptor getters executed user code. Reentrant mutation of the target could resize the flat hash and leave the retained pointer dangling. Descriptor getters were also evaluated again after fast-array conversion. The fix is to convert the descriptor before querying the target and to place the retry point after descriptor conversion. The issue was introduced in 2f288a9 (0.9.1). This closes nginx#1089 issue on GitHub.
Previously, PCRE name-table capture indexes were decoded through plain char. On signed-char targets, bytes with the high bit set were sign-extended and produced a wild or incorrect capture index used to read a match array. The fix is to decode unsigned bytes through a checked output parameter and to validate every named capture index against the compiled capture count. The issue was introduced in 9496944 (0.3.2). This closes nginx#1088 issue on GitHub.
Previously, integer-indexed exotic validation was restricted to keys marked as strings. Numeric atoms and the canonical string "NaN" could bypass the check and be installed as ordinary typed-array properties. The fix is to validate numeric atoms and canonical numeric strings after key conversion, and to remove the obsolete source-type flag. The numeric-key bypass was introduced in b28e50b (0.9.0); the incomplete canonical-index handling dates to bf20804 (0.3.8). This closes nginx#1098 issue on GitHub.
Previously, njs_value_to_primitive() retained a pointer to the caller's value slot while valueOf(), toString(), or property getters executed user code. Reentrant code could overwrite that slot, after which the conversion treated the replacement primitive as the original object. The fix is to snapshot the input value before the first observable property lookup and use the stable copy throughout conversion. The issue was introduced in 64dbf10 (0.3.4). This closes nginx#1094 issue on GitHub.
Previously, sort deleted every index from the compacted output position to the original length. A sparse array with length near 2^32 therefore caused billions of property-delete operations and unbounded memory growth. The fix is to retain the direct cleanup loop for fast arrays and, for slow arrays, enumerate and delete only existing indexed properties in the range. The issue was introduced in 1a2a65f (0.8.0). This closes nginx#1095 issue on GitHub.
xeioex
force-pushed
the
fuzzer-issues
branch
from
September 4, 2026 21:04
98cb807 to
17ecb8b
Compare
Contributor
Author
|
Resolved the conflict in src/njs_regexp.c by combining:
|
github-project-automation
Bot
moved this from In Review
to Done
in NGINX OSS Unified Workspace
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
2 participants
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.
Overall, 11 fuzzer reports were reduced to 6 root causes and addressed by 8 orthogonal patches.