Re: speculative buffer overrun in SpiderMonkey

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.

I haven't properly debugged my changes, I only tested by adding logging to
the functions I modified to checked that some (don't recall which now) are
actually called when I access array elements from javascript in Firefox.
I don't understand yet the structure of the code and I still need to learn
testing setups, I hope to advance on this in my spare time.
I'll take a look at the core reference you mention.

Thanks,
llongeri


On Tue, Jan 16, 2018 at 12:36 PM, David Teller <[email protected]> wrote:

>
>
> On 16/01/2018 15:16, Luis Longeri wrote:
> > I just limit the indexing using a modulo operator, since the MOZ_ASSERT
> > check could be delayed by the CPU by a cache miss, I am enforcing a limit
> > on the index so if the indexing runs speculatively it will still be
> within
> > limits prior to being discarded. These indexing functions are called from
> > places where the index is checked with 'if' statements but those
> > evaluations can also be delayed by the CPU allowing for an speculative
> > execution of an overflow.
>
> I suspect that you're not testing what you intend to test. By design,
> MOZ_ASSERT code is only executed in DEBUG builds, so it's not something
> that can be exploited in a Meltdown/Specter scenario.
>
> I *think* that the code you're looking for is actually in
> `NativeObject::getSlot`.
>
> Cheers,
>  David
>