dylibso / observe-sdk

Continuous runtime observablity SDKs to monitor WebAssembly code.
https://dev.dylibso.com/docs/observe/overview
Apache License 2.0
157 stars 7 forks source link

fix: js name parsing - reading subsection ids >= 128 and error handling #79

Closed G4Vi closed 1 year ago

G4Vi commented 1 year ago

Wrapping most of it with a try catch created an unfortunate diff.

This improves the reliability and accuracy of the name parsing:

  1. Reads the name subsection id as a byte so ids >= 128 are supported.
  2. Wraps nameSectionView accesses with try...catch so out of bound accesses (RangeError) will just cease parsing of name section and return the names with successfully parsed before that point.
  3. Only reads up to nameMapLength.value names from a subsection instead of relying it the namemap going until the end of the subsection. If there's leftover bytes in a subsection, they are skipped, and a warning is logged.
wikiwong commented 1 year ago

Great catch, I thought you could read regular integers as LEB128 as well but good to know that can cause issues!

Out of curiosity, did you encounter an issue, or was this something you just realized while looking at the code?

G4Vi commented 1 year ago

Updated PR description.

Great catch, I thought you could read regular integers as LEB128 as well but good to know that can cause issues!

With LEB128 parsing If the high bit was set on the subsection id, part of the subsection length would accidentally be parsed into it. Right now I don't think anyone's used >= 128 subsection ids in the name section, but nothing is stopping them.

Out of curiosity, did you encounter an issue, or was this something you just realized while looking at the code?

I didn't encounter any issue, I just heard you mention reading the subsection id as a LEB128 in your teamlog so I dug into the code to see if it was left like that. The other changes are just paranoia about corrupt modules being passed into the observe sdk and it being tricky to track down.

wikiwong commented 1 year ago

Great work, thank you!