Re: speculative buffer overrun in SpiderMonkey

On 1/21/18 7:30 AM, Luis Longeri wrote:
> Thanks, I suspected that MOZ_ASSERT was only for DEBUG mode, but I didn't
> check it. As an assert, I understand it should never assert in bug free JS
> engine, so the value of the index is already checked by the time MOZ_ASSERT
> is executed in DEBUG (or skipped in production build).
> But the code I patched, such as the function getDenseElement, is called for
> example from HasAndGetElement or GetArrayElement (in js/src/jsarray.cpp)
> such as:
> 
> if (index < nobj->getDenseInitializedLength()) {
>              vp.set(nobj->getDenseElement(size_t(index)));
> 
> That IF statement is the branching that could trigger a speculative
> execution of the getDenseElement function if index is greater or equal to
> the initialized length.


Yes, that looks like an example of a spectre-vulnerable computation.

I don't think modulus is a good fix, though; you have to do a division, 
which I think can take a number of cycles and tie up an ALU or division 
unit. It would be better to mask, though that means calculating or 
maintaining a mask value. (And it isn't precise; the attacker could 
snoop nearby data.)

It looks like the bug for this is 
https://bugzilla.mozilla.org/show_bug.cgi?id=1430051 which has other ideas.