Skip to content

Fix use-after-free resolving ref types after instruction recovery - #2823

Open
aizu-m wants to merge 2 commits into
WebAssembly:mainfrom
aizu-m:ref-type-uaf-recovered-instr
Open

Fix use-after-free resolving ref types after instruction recovery#2823
aizu-m wants to merge 2 commits into
WebAssembly:mainfrom
aizu-m:ref-type-uaf-recovered-instr

Conversation

@aizu-m

@aizu-m aizu-m commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

a block/loop/if whose signature names a reference type defers a resolution pointing into it and is freed when it fails to parse, but ParseInstrList recovers from that failure so the enclosing func still parses and the #2805 field-level truncation never runs, leaving the resolution to dereference the freed instruction; truncate the resolve lists on the instruction recovery path (ParseInstrList/ParseExprList) too, the same way ParseModuleField already does on a failed field.

@sbc100
sbc100 requested a review from zherczeg August 14, 2026 17:45

@sbc100 sbc100 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable to me

@sbc100

sbc100 commented Aug 14, 2026

Copy link
Copy Markdown
Member

Out of interest @aizu-m, why are you so interested in fixed these types of bugs in wabt?

@zherczeg zherczeg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are both changes covered by a single test?

Comment thread src/wast-parser.cc Outdated
void WastParser::TruncateResolveLists(size_t ref_types_size,
size_t type_vectors_size,
size_t funcs_size) {
resolve_ref_types_.erase(resolve_ref_types_.begin() + ref_types_size,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about using vector::resize?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, switched all three to resize (needed to add defaulted constructors to the resolve structs since they only had value ctors). Reads better.

@zherczeg

Copy link
Copy Markdown
Collaborator

Unrelated to this PR, but it is also a good question how can we manually maintain these reverts. How can we get some info that it is missing somewhere.

@aizu-m

aizu-m commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

@zherczeg on the test: before, only the ParseInstrList path was hit (the two bare block/loop funcs). The ParseExprList branch was untouched, so I added a folded case to the same test: (func (drop (if (result (ref $t)) nop) (nop))). The if registers a deferred resolution for its signature then fails looking for (then ...), and recovery happens in ParseExprList since it's an operand of a folded drop. Confirmed it: with only the ParseExprList truncation reverted, that line alone ASan-aborts in ResolveTargetTypeVector, and it's clean with the fix. So both paths are now covered by the one file.

@sbc100 mostly curiosity plus it's good practice. I've been fuzzing the wat/binary front-ends (libFuzzer + ASan/UBSan) and these parser recovery paths keep turning up reachable use-after-frees, so I've been sending fixes as I minimise them. This one fell out of the same corpus as #2805.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants