speculative buffer overrun in SpiderMonkey

Hi,

Last week I posted this on firefox-dev and I was advised to re-post here.

When I stumbled into the Mealtdown and Spectre exploits news I jump into
reading the papers, didn't sleep much. So I decided to see what could be
done to get better Javascript protection against speculative buffer
overruns.

I have wanted to check this out for some time, so at last I downloaded
Firefox source code and I started to figure out if some protections could
be added. This is just dev poking around and I would like to ask if I am at
least in the right direction.

I briefly when over the code and I figure that NativeObject.h seems to
declare array indexing code used at least from the Javascript interpreter.
So I made the following changes that I understand should cut short any
speculative buffer overrun at least in these patched functions. I know this
has performance issues but it is just a test.

I compiled this and I am running Firefox in safe mode (from what I gather
this disables the jit compiler which I haven't even begin to look at).
From the limited testing I have done, this code is effectively called when
indexing arrays.
Right now I am using this patch in safe mode in a test computer, so far it
works okay but of course a little slow on sites heavy on javascript like
google drive.

I would like to ask if at least I am in the right direction or am I way off
course.

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.

diff -r f78a83244fbe js/src/vm/NativeObject.h
--- a/js/src/vm/NativeObject.h    Thu Jan 04 11:44:30 2018 +0200
+++ b/js/src/vm/NativeObject.h    Thu Jan 04 16:05:11 2018 -0300
@@ -496,11 +496,13 @@
         return HeapSlotArray(elements_, true);
     }
     const Value& getDenseElement(uint32_t idx) const {
-        MOZ_ASSERT(idx < getDenseInitializedLength());
-        return elements_[idx];
+        uint32_t len = getDenseInitializedLength();
+        MOZ_ASSERT(idx < len);
+        return elements_[idx % len];
     }
     bool containsDenseElement(uint32_t idx) {
-        return idx < getDenseInitializedLength() &&
!elements_[idx].isMagic(JS_ELEMENTS_HOLE);
+        uint32_t len = getDenseInitializedLength();
+        return idx < len && !elements_[idx % len].isMagic(JS_ELEMENTS_HOLE)
;
     }
     uint32_t getDenseInitializedLength() const {
         return getElementsHeader()->initializedLength;
@@ -1196,9 +1198,11 @@
     // objects, but should only be called in a few places, and should be
     // audited carefully!
     void setDenseElementUnchecked(uint32_t index, const Value& val) {
-        MOZ_ASSERT(index < getDenseInitializedLength());
+        uint32_t len = getDenseInitializedLength();
+        MOZ_ASSERT(index < len);
         MOZ_ASSERT(!denseElementsAreCopyOnWrite());
         checkStoredValue(val);
+        index %= len;
         elements_[index].set(this, HeapSlot::Element,
unshiftedIndex(index), val);
     }

@@ -1217,10 +1221,12 @@
     }

     void initDenseElement(uint32_t index, const Value& val) {
-        MOZ_ASSERT(index < getDenseInitializedLength());
+        uint32_t len = getDenseInitializedLength();
+        MOZ_ASSERT(index < len);
         MOZ_ASSERT(!denseElementsAreCopyOnWrite());
         MOZ_ASSERT(!denseElementsAreFrozen());
         checkStoredValue(val);
+        index %= len;
         elements_[index].init(this, HeapSlot::Element,
unshiftedIndex(index), val);
     }

Regards,
llongeri