1281194 - for-of iteration over module import list in ResolveRequestedModules is visible to web pages

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Consider a page that does this:

1)  Run a script that sets [].values().__proto__.next to a function it controls.
2)  Load a module with some imports.

We will end up in ResolveRequestedModules in nsScriptLoader.cpp doing a for-of loop over the return value of JS::GetRequestedModules.  But that will invoke the page-controlled function on the array iterator prototype, for what I'm told is an implementation detail.  That seems bad.

What we should probably do here is use a loop from 0 to length and JS_GetElement instead (especially since we're already getting the array length but not using it for anything right now).  I _think_ that should be safe.  If we want to be very sure it's safe, we can use something like the self-hosted List() type: an object with null prototype, a "length" property, and indexed props.  That will ensure that no one can observe the gets.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1281194-array-iteration

As suggested, use JS_GetElement rather than for-of iteration.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8764273 [details] [diff] [review]
bug1281194-array-iteration

r=me though it might make sense to make the comments a bit more dire about how you should not trust anything other than going 0 to length-1.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2eb6f3b5c5e
Don't use for-of iteration over requested modules array r=bz

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b2eb6f3b5c5e