1670913 - DOM Sanitizer API should parse into a throw-away "loaded as data" document

Status: RESOLVED FIXED

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

Description

[Henri Sivonen (:hsivonen)]

Currently, dom/security/sanitizer/Sanitizer.cpp uses a DocumentFragment whose owner is the document of the script global.

To avoid initiating fetches for img/audio/etc. src if the node gets removed by the sanitizer, the DocumentFragment used as parsing target should belong to a Document that has its "loaded as data" bit set.

Comment 1

[Henri Sivonen (:hsivonen)]

The relevant Web-exposed API is preffed off, but the same problem applies elsewhere, too. Let's keep this bug hidden as long as the related bugs are.

Comment 2

[Frederik Braun [:freddy]]

I can take it, but I'm going to have questions down the line. Thanks for filing, Henri!

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1652481

Comment 4

[Henri Sivonen (:hsivonen)]

(In reply to Frederik Braun [:freddy] from comment #2)

I can take it, but I'm going to have questions down the line. Thanks for filing, Henri!

Bug 1666300 part 2 provides a function that will be of use here.

Comment 5

[Frederik Braun [:freddy]]

Masato Kinugawa (now in CC) found the same thing.
The Sanitizer API will make requests when parsing markup with external resources and really shouldn't.
E.g., (new Sanitizer()).sanitize("<img src='https://example.com'></img>")

Comment 6

[Frederik Braun [:freddy]]

Attachment: Bug 1670913 - use inert document within Sanitizer API r?hsivonen

Comment 7

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

Landed: https://hg.mozilla.org/integration/autoland/rev/3907dd86dd7151339fa556b5395bb8e12a6afc59

Backed out because it was not intended to land: https://hg.mozilla.org/integration/autoland/rev/1659fc495cb441a8e2b37ff835842797149f2a5f

Comment 8

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

use inert document within Sanitizer API r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/eb9d5a5e536db27799161a7883a925a979411d3a
https://hg.mozilla.org/mozilla-central/rev/eb9d5a5e536d

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:freddy, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Frederik Braun [:freddy]]

It's a disabled configuration and we're explicitly asking for folks to test on Nightly. I don't think the uplift is justified.
I'm wondering if a status of "disabled" is better than "wontfix" for the general analyses? Both apply here and I'm taking the bots recommendation.

Comment 11

[Brindusa Tot, Desktop QA]

Could this be manualy verified? If yes, please help us with some specific steps or a testcase.

Comment 12

[Frederik Braun [:freddy]]

No need, the patch comes with an automated test.

Comment 14

[Tom Ritter [:tjr]]

Actually, if this is disabled in release we don't need an advisory.