Step 4 (t0 = x^0xc) passed x^1 (&xx) instead of x^3 (out1), producing
an incorrect exponent and rejecting valid quadratic residues. This broke
compressed SEC1 P-256 point decoding via pt_set_sec_bytes.
unquote_string replaces each byte that is not valid UTF-8 with U+FFFD,
which is three bytes for one, but sized its buffer as len(s) + 2*UTF_MAX:
slack for a single replacement, not for one per invalid byte. A string
holding several ran the write cursor past the end, an out-of-range slice
under bounds checking and a memory-safety bug without it.
Count the invalid bytes in the remainder up front and size for them. The
escape sequences never grow their input, so they need no allowance.
`check_scope_decls` iterates a scope's entity map while
`check_entity_decl` can insert into that same scope: a procedure alias
(`f :: e`) goes through `override_entity_in_scope`, which calls
`scope_map_insert`. The map grows at 75% load (12 of 16 slots, then 24
of 32), and `scope_map_insert` grows before it probes for the key, so
even an alias that only replaces an existing value rehashes and
reallocates the table in the middle of the iteration.
`ScopeMapIterator` re-read its map on every step while `end` had
captured the capacity at loop start, so the walk continued over the new
table under the old sentinel: it ran past the end of the reallocated
table and dereferenced whatever followed it as an `Entity *` - a nullptr
dereference for a procedure scope holding exactly 12 or 24 entities - or
it stopped at the stale end index and silently skipped every declaration
that the rehash had moved behind it, which left unused declarations
without any diagnostic at all.
The iterator now snapshots the table pointer and capacity when it is
constructed, so iteration is invariant under any insert or grow and
cannot leave the table it began on. A scope that does not grow behaves
exactly as before: the snapshot points at the same table.
Refs #7598
Checks that the backing memory of the scratch
allocator gets reused upon memory depletion.
Checks that the backing allocator only gets used
when allocations bigger than the backing memory are
requested.
PR #6476 "fixed" an issue that didn't exist because it misunderstood this allocator's semantics. What it conluded was an edge case was in fact entirely predictable and desireable behavior for this allocator.
In "fixing" this edge case, it prevented the exact scratch mechanics that sets this allocator apart for its use case.
Reverted.
parse_object_body allocates an object key, then may fail in parse_colon or
parse_value before that key is ever inserted into the object. Its cleanup defer
only walks `obj`, so a key that never got there is unreachable to it. The caller
cannot free it either -- a failed parse returns a nil Value -- so it leaks.
The same applies to the parsed element on the duplicate-key path, and to both on
the out-of-memory path.
JSON5 makes this reachable from ordinary malformed input, because an unquoted
ident is a legal key and anything other than a colon after it fails. Plain JSON
leaks it too, via a quoted key.
before, measured with a tracking allocator over 8 inputs x 2 specs:
LEAK JSON5 colon fails after unquoted key 1 alloc / 7 bytes
LEAK JSON colon fails after quoted key 1 alloc / 2 bytes
LEAK JSON5 colon fails after quoted key 1 alloc / 2 bytes
LEAK JSON value fails after key 1 alloc / 2 bytes
LEAK JSON5 value fails after key 1 alloc / 2 bytes
LEAK JSON nested value fails 2 alloc / 4 bytes
LEAK JSON5 nested value fails 2 alloc / 4 bytes
LEAK JSON deep nesting fails 3 alloc / 6 bytes
LEAK JSON5 deep nesting fails 3 alloc / 6 bytes
LEAK JSON array element fails 1 alloc / 2 bytes
LEAK JSON5 array element fails 1 alloc / 2 bytes
total leaked allocations: 17
after, same probe:
total leaked allocations: 0
The leak scales with nesting depth -- one orphaned key per enclosing object -- so
a service parsing untrusted JSON leaks a little on every malformed request.
The fix marks the key and the element as owned by the loop iteration until they
are stored, and frees them otherwise. The duplicate-key path loses its explicit
delete, which the same mechanism now covers.
Found via odinfmt, which reported a 7-byte leak in a downstream test that parses
`{ broken not json` to check that invalid input is rejected.
Regression test added to tests/core/encoding/json: it reports
`17 leaks and 0 bad frees` without this change and passes with it. The existing
11 tests pass unchanged under -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true.
Tagless switch cases are ordered predicates, so distinct constant conditions may fold to the same Boolean value. Restrict duplicate-case tracking to explicitly tagged switches and add positive and negative regression coverage for issue #7421.
i was working on this when i stumbled on this bug, so i did bunch of different characters to see which ones cause issue, hence test has many different options. I could make it either simpler or split it, as needed.
Ihor Antonov