Skip to content

reject out-of-range sourceIndex in source map parser - #2253

Open
Medios-lgtm wants to merge 1 commit into
meta:static_hfrom
Medios-lgtm:sourcemap-sourceindex-bounds
Open

Medios-lgtm wants to merge 1 commit into
meta:static_hfrom
Medios-lgtm:sourcemap-sourceindex-bounds

Conversation

@Medios-lgtm

Copy link
Copy Markdown
Contributor

Summary

SourceMapParser builds each segment's sourceIndex by accumulating a delta-encoded VLQ value, but never checks it against the number of entries in the map's sources array. The comment in SourceMap::getLocationForAddressFIndex claims parseSegment() validated this, but nothing did. SourceMap::getLocationForAddress passes the value to getSourceFullPath, which indexes sources_ directly behind only a debug-only assert.

A source map whose mapping segment names an out-of-range (or negative) sourceIndex therefore reads sources_ out of bounds once a location is queried, for example when SourceMapTranslator::translate maps a diagnostic back through an attached input source map. In a release build the assert is compiled out and the wild std::string becomes a StringRef used as a file name.

This validates sourceIndex while parsing the mappings and rejects the map if any segment is out of range, which is where the stale comment already expected the check to live. nameIndex is not validated because it indexes no array at query time (it is only re-emitted by the generator).

The unittests/SourceMap directory was never wired into unittests/CMakeLists.txt, so its tests did not build or run; this adds the add_subdirectory line and a regression test alongside it.

Test Plan

AgqjGAA decodes to sourceIndex 100000 and ADAA to -1, against a map with a single source. Before the change, compiling the standalone parser with -DNDEBUG -fsanitize=address and calling getLocationForAddress(1, 1) reports:

AddressSanitizer: ... READ memory access
  #5 hermes::SourceMap::getSourceFullPath(unsigned int) const SourceMap.h:185
  #6 hermes::SourceMap::getLocationForAddress(unsigned int, unsigned int) const SourceMap.cpp:74

After the change both maps are rejected by the parser and a valid map still resolves its source. New test SourceMap.OutOfRangeSourceIndex fails before the fix and passes after; the full HermesSourceMapTests suite (14 tests) passes, and the hermes target builds clean under the ASan+Debug configuration.

Signed-off-by: sabith saheb <admin@medios.digiscrypt.com>
@meta-cla

meta-cla Bot commented Oct 17, 2026

Copy link
Copy Markdown

Hi @Medios-lgtm!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@meta-cla

meta-cla Bot commented Oct 17, 2026

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 17, 2026

This branch has not been deployed

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

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant