[PATCH] D50387: [WASM] Fix overflow when reading custom section

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: llvm/lib/Object/WasmObjectFile.cpp:222
+    const uint32_t NameLength = Ctx.Ptr - NameStart;
+    if (NameLength > Size)
+      return make_error<StringError>(
----------------
JDevlieghere wrote:
> sbc100 wrote:
> > JDevlieghere wrote:
> > > Can this section be empty? (i.e. should I make this greater or equal)
> > I think the ReadContext is probably a better way to enforce this.  readString already does this check based on the context.
> > 
> > This check seems a little strange since it reads the string before checking if its too long.
> > 
> I started out by doing that that but then you still have to update the current pointer and the size which made the code less intuitive. That combined with a less readable error message made me go this route. I'm happy to change it if you still think the ReadContext is better given the context. (pun not intended :-) 
I don't particularly care about the precision and readability of the errors we generate on malformed input.   Having a specific error message for this one strange case actually seems like overkill.  However, if this is the most readable solution I think I'd be ok with it.  

What did you earlier attempt look like.  I would suggest creating an entirely new Context on the stack just for reading this Name.  Is that less readable? 

I'm happy to change the error massage to say "out of bounds" or something like that rather than "EOF" too.


Repository:
  rL LLVM

https://reviews.llvm.org/D50387