[Webkit-unassigned] [Bug 147393] Parse the entire WebAssembly modules

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=147393

--- Comment #9 from Sukolsak Sakshuwong <sukolsak at gmail.com> ---
Thanks.

(In reply to comment #8)
> Comment on attachment 257910 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257910&action=review
> 
> > Source/JavaScriptCore/wasm/WASMReader.cpp:83
> > +    CHECK_READ(1);
> 
> Why not do everything in the loop?

It's just an optimization for small integers. If this seems like a micro-optimization, let me know and I will fix it.

> > Source/JavaScriptCore/wasm/WASMReader.cpp:89
> > +    while (true) {
> 
> What's stopping this from reading more than 4 bytes?
> 
> > Source/JavaScriptCore/wasm/WASMReader.cpp:96
> > +        result |= (byte & 0x7f) << shift;
> 
> Or maybe the limit is we can read at most 5 bytes?
> Can compact int values be larger than 2^28?

My first thought was that users couldn't do anything malicious with this anyway, because we always used CHECK_READ(), and unsigned integer overflow is well-defined. But I can see why this can be a problem. So, should we limit the number of bytes?

Compact (LEB128) uint32 values can be up to 2^32 - 1. So, the limit should be 5 bytes. If we check the number of bytes, should we check as well that the fifth byte is less than 2^(32-28) = 2^4 to prevent an integer overflow?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150731/0f5cbd09/attachment.html>