1021312 - Zone Mismatch in CloneNonReflectors

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

CloneNonReflectors currently assumes that it only needs to worry about compartments in the v.isObject() case. However, we can still get mismatches with cross-zone strings. The solution is to wrap the string, which causes non-interned strings to be copied into the target zone.

The mismatch is mitigated to sec-moderate because this can only be triggered by a privileged API that doesn't currently have any in-tree consumers.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Part 1 - Wrap strings in CloneNonReflectors. v1

Comment 2

[Bobby Holley (:bholley)]

Attachment: Part 2 - use a fresh zone for xpcshell. v1

Comment 3

[Bobby Holley (:bholley)]

Attachment: Part 3 - Tests. v1

Comment 4

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=0872c62a17e2

Comment 5

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=8501686e4923

Comment 6

[Bobby Holley (:bholley)]

Comment on attachment 8435350 [details] [diff] [review]
Part 1 - Wrap strings in CloneNonReflectors. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): the introduction of exportFunction - longstanding
User impact if declined: potential memory hazard
Testing completed (on m-c, etc.): just pushed to m-i
Risk to taking this patch (and alternatives if risky): Extremely low risk.
String or IDL/UUID changes made by this patch: None

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1ab8174e097f
https://hg.mozilla.org/mozilla-central/rev/08045ba9192f
https://hg.mozilla.org/mozilla-central/rev/340e016163ed

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9bcea7802320
https://hg.mozilla.org/releases/mozilla-aurora/rev/2b3f51c3c5a7
https://hg.mozilla.org/releases/mozilla-aurora/rev/999c34fa59a2

Should we consider taking this on b2g30 for v1.4 as well?

Comment 9

[Bobby Holley (:bholley)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #8)
> Should we consider taking this on b2g30 for v1.4 as well?

Naw. Bug 1021244 is the first in-tree usage of this outside of testing, so the uplift is primarily concerned with addons (which aren't a problem for b2g).

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Backed out from Aurora for failures in the new test.
https://hg.mozilla.org/releases/mozilla-aurora/rev/95deb57e283b

https://tbpl.mozilla.org/php/getParsedLog.php?id=41223994&tree=Mozilla-Aurora

Comment 11

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/965578443062

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a5967705754e
https://hg.mozilla.org/releases/mozilla-aurora/rev/68ee1d33fc83
https://hg.mozilla.org/releases/mozilla-aurora/rev/b982a8afab52

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out along with bug 1020460 in https://hg.mozilla.org/integration/mozilla-inbound/rev/6502442e6133

Relanded in https://hg.mozilla.org/integration/mozilla-inbound/rev/c0f0a8689dab since it wasn't part of the bustage.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Matt Wobensmith [:mwobensmith][:matt:]]

Marking [qa-] in the absence of test media or STR, please feel free to provide if you'd like help testing.