Fix out-of-bounds read in GetNameSectionSubsectionName - #2801
Conversation
NameSectionSubsection is a signed enum with no explicit underlying type, so a name subsection id >= 0x80000000 casts to a negative value that passes the `type <= Last` gate in ReadNameSection and indexes NameSubsectionName out of bounds when logged. Range-check the index like the sibling accessors GetKindName and GetRelocTypeName.
| }; | ||
| // clang-format on | ||
|
|
||
| const char* GetNameSectionSubsectionName(NameSectionSubsection subsec) { |
There was a problem hiding this comment.
Can we instead have the caller of this function (who creates the NameSectionSubsection from bytes) ensure that the subsec is valid?
Either that or have this function take an integer type rather then a NameSectionSubsection?
Basically I think its function take a enum as an argument it should a pre-condition that the enum is in-range.
There was a problem hiding this comment.
(In this case, an assert should still be added here)
There was a problem hiding this comment.
Moved the check to the caller. ReadNameSection now compares the raw name_type before the cast, so an id past Last is skipped as an unknown subsection and never becomes a NameSectionSubsection in the first place (the cast itself was unspecified for those values anyway). The accessor is back to a plain index with an assert on the precondition, per @zherczeg.
Checked against the original module (subsection type LEB 0xffffffff): the assert fires with the old reader, and wasm2wat -v runs clean with the new one. Kept the regression test, it still covers that path.
Reject an out-of-range subsection id before it is cast, so GetNameSectionSubsectionName is only ever handed an in-range enum, and assert that precondition there rather than returning an error string.
| // table when it is logged. | ||
| Result result = ReadBinary(data, &reader, options); | ||
| (void)result; | ||
| } |
There was a problem hiding this comment.
Can this be a normal file-based test like name-section-location.txt?
3 participants
ASan,
wasm2wat -von a valid module whose name section carries oneunknown subsection (type LEB
0xffffffff):Worked back from the read to the subsection id.
NameSectionSubsectionhas no explicit underlying type, so it is a signed int.
ReadNameSectionreads the id into a
uint32_t, casts it, and only gates theOnNameSubsectioncallback withtype <= Last, a signed compare with nolower bound. An id of
0xffffffffcasts to a negative enum, passes thegate, and reaches
GetNameSectionSubsectionName, which indexedNameSubsectionName[size_t(subsec)]with no range check. The loggingdelegate then prints the returned wild
const char*with%s.The module is valid: an unknown name subsection is skipped, so this only
shows with verbose logging (
wasm2wat -v,wasm-objdump -v,wasm-interp -v). Found feeding fuzz-style name sections throughwasm2wat -v.Fixed in the caller:
ReadNameSectioncompares the rawname_typeagainst
Lastbefore the cast, so an out-of-range id is skipped as anunknown subsection and never becomes a
NameSectionSubsection.GetNameSectionSubsectionNamekeeps the plain index and asserts thein-range precondition. Regression test added under
src/test-binary-reader.cc.