468045 - worker threads can access Components.*

Status: RESOLVED FIXED

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

Description

[timeless]

Attachment: script

Components.stack/Components.classes are supposed to be restricted from content

Components.interfaces is supposed to be available (for historical reasons).

Comment 1

[timeless]

Attachment: page

Comment 2

[timeless]

Attachment: page

Comment 3

[Hixie (not reading bugmail)]

No need to let worker threads access Components.interfaces either, there's no legacy here.

In fact, we shouldn't expose Components at all.

Comment 4

[timeless]

i've given ted a patch to try, if it works, i'd hope someone will commit it

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: timeless' patch

timeless asked me to test this patch he wrote. Without this patch, I see the contents of Components.classes in the test page here, with the patch I just see "hiReferenceError: Components is not defined".

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Ugh, this was a todo that got forgotten... I'm sort of amazed that this works despite the JS_DeleteProperty. Clearly we need to check in a test case here.

Comment 7

[Giorgio Maone [:ma1]]

On a side note, it seems that Worker is not subject to CAPS property access checks.

user_pref("capability.policy.default.XMLHttpRequest.send", "noAccess");

works,

user_pref("capability.policy.default.Worker.postMessage", "noAccess");

doesn't.

Should we file a separate bug?

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

That has nothing to do with accessing the Components object, so yes :)

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Support for that is more or less dropped everywhere, so I don't think there's need to do anything. However if you do think we should support it, or have a discussion about it, please file a separate bug or bring it to the newsgroup. It's definitely unrelated to this bug.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 351542 [details] [diff] [review]
timeless' patch

Looks good!

Comment 11

[(not currently active) Ted Mielczarek]

bent: can you write a test for this? I'll push this change ASAP.

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #6)
> I'm sort of amazed that this works despite the JS_DeleteProperty.

Ugh, it's defined with JSPROP_PERMANENT. Patch with testcase coming up in a sec.

Comment 13

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch for checkin

Comment 14

[Bob Clary [:bc] (inactive)]

(In reply to comment #13)
> Created an attachment (id=351606) [details]
> Patch for checkin

How does the test's mention of Components alert watchers that there may be a security issue to be exploited? Can we check this test in while we are shipping vulnerable releases?

Comment 15

[Boris Zbarsky [:bzbarsky]]

We should be checking the test in, yes.  None of our stable releases are in fact vulnerable.

Comment 16

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Pushed changeset 3d7f812792fb to mozilla-central.

Comment 17

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Pushed changeset 92994c8da343 to mozilla-1.9.1.