1702608 - nsDSURIContentListener::DoContent's use of mDocShell is suspicious

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Olli Pettay [:smaug][[e-mail]]]

mDocShell is a raw pointer[1], yet it is used to call methods which may run scripts [2].

[1] https://searchfox.org/mozilla-central/rev/4fa18c26fa907f38d56b599571b9846af1506f3c/docshell/base/nsDSURIContentListener.h#87
[2] https://searchfox.org/mozilla-central/rev/4fa18c26fa907f38d56b599571b9846af1506f3c/docshell/base/nsDSURIContentListener.cpp#156,178

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Bug 67721 added effectively a null check for mDocShell later in the method.
mDocShell will be null there, but while mDocShell is used to call some method, nothing seems to keep it alive.

And bug 523260 is possibly related to this bug.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1702608, ensure the same docshell is used in nsDSURIContentListener, r=kmag

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9213170 [details]
Bug 1702608, ensure the same docshell is used in nsDSURIContentListener, r=kmag

Security Approval Request

• How easily could an exploit be constructed based on the patch?: There is no known exploit, but the change does pinpoint a bit where an issue might be.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: (The relevant code up to 78 looks the same https://searchfox.org/mozilla-esr78/source/docshell/base/nsDSURIContentListener.cpp#119)
• How likely is this patch to cause regressions; how much testing does it need?: Should be very safe. Unlikely to cause regressions.

Comment 4

[Tom Ritter [:tjr]]

I'm going to let this one sit a little bit longer until after pwn2own.

Comment 5

[Tom Ritter [:tjr]]

Comment on attachment 9213170 [details]
Bug 1702608, ensure the same docshell is used in nsDSURIContentListener, r=kmag

Approved to land and uplift

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

ensure the same docshell is used in nsDSURIContentListener, r=kmag
https://hg.mozilla.org/integration/autoland/rev/6dcf9156484f47f390e57819340fe948396b803d
https://hg.mozilla.org/mozilla-central/rev/6dcf9156484f

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/75d2691ada4b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/b854ced3e4bc