Closed Bug 1373042 Opened 5 years ago Closed 2 years ago

ExportHelpers has a nice long comment about FileList that doesn't match reality

Categories

(Core :: XPConnect, enhancement, P3)

53 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox-esr68 --- wontfix
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: sec-audit, Whiteboard: [post-critsmash-triage][adv-main76-][adv-ESR68.8-])

Attachments

(1 file)

The comment at talks about how we can rely on FileList being implemented as an XPCWN and so on, but they haven't been XPCWN in a while.  Sadly, UnwrapReflectorToISupports() still works on Web IDL objects, so this codepath is still returning true for FileList.

Does that mean we're doing unsafe things when mOptions->wrapReflectors is false?  Should we just remove this FileList special-case?  Something else?
Flags: needinfo?(bobbyholley)
See Also: → CVE-2017-7801
Summary: ExportHelpers has a nice long column about FileList that doesn't match reality → ExportHelpers has a nice long comment about FileList that doesn't match reality
Nice catch - I think this is ok though. The impact of the bug is that we'll effectively always have the wrapReflectors=true behavior for FileList, which potentially means that somebody cloning an object graph containing a FileList might get have the error appear lazily (via a security wrapper denial when content tries to access the object) rather than eagerly (when the clone happens).

In other words, security wrappers save us. So the safest thing is just to leave this be for the next two cycles and then remove the special case once XPCOM addons go away.
Flags: needinfo?(bobbyholley)
Priority: -- → P3
Do we have a tracking bug for things we can do post-57?
Flags: needinfo?(overholt)
Flags: needinfo?(overholt)
I searched for at least 15 minutes and couldn't find bug 1347507. Thanks, Andrew!
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main76-][adv-ESR68.8-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.