1673164 - received invites show just a blank page. And perma comm/calendar/test/unit/test_ltninvitationutils.js | xpcshell return code: 0 (also comm/calendar/test/browser/invitations/browser_imipBarEmail.js)

Status: RESOLVED FIXED

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

Description

[Treeherder Bug Filer]

Filed by: mkmelin+mozilla [at] iki.fi
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=319597138&repo=comm-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Ve3GCgwoQOa_2JvSHYmWvQ/runs/0/artifacts/public/logs/live_backing.log

---

[task 2020-10-23T23:03:06.987Z] 23:03:06     INFO -  xpcshell-icaljs.ini:comm/calendar/test/unit/test_ltninvitationutils.js | Starting createInvitationOverlay_test
[task 2020-10-23T23:03:06.987Z] 23:03:06     INFO -  (xpcshell/head.js) | test createInvitationOverlay_test pending (2)
[task 2020-10-23T23:03:06.988Z] 23:03:06     INFO -  PID 23054 | [23054, Main Thread] WARNING: EnsureInit: No screen: file /builds/worker/checkouts/gecko/widget/gtk/nsLookAndFeel.cpp:1036
[task 2020-10-23T23:03:06.988Z] 23:03:06     INFO -  PID 23054 | [23054, Main Thread] WARNING: EnsureInit: No screen: file /builds/worker/checkouts/gecko/widget/gtk/nsLookAndFeel.cpp:1036
[task 2020-10-23T23:03:06.988Z] 23:03:06     INFO -  PID 23054 | [23054, Main Thread] WARNING: EnsureInit: No screen: file /builds/worker/checkouts/gecko/widget/gtk/nsLookAndFeel.cpp:1036
[task 2020-10-23T23:03:06.989Z] 23:03:06     INFO -  PID 23054 | [23054, Main Thread] WARNING: EnsureInit: No screen: file /builds/worker/checkouts/gecko/widget/gtk/nsLookAndFeel.cpp:1036
[task 2020-10-23T23:03:06.989Z] 23:03:06     INFO -  PID 23054 | [23054, Main Thread] WARNING: EnsureInit: No screen: file /builds/worker/checkouts/gecko/widget/gtk/nsLookAndFeel.cpp:1036
[task 2020-10-23T23:03:06.990Z] 23:03:06     INFO -  PID 23054 | [23054, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /builds/worker/checkouts/gecko/dom/base/ThirdPartyUtil.cpp:402
[task 2020-10-23T23:03:06.990Z] 23:03:06     INFO -  PID 23054 | [23054, Main Thread] WARNING: NS_ENSURE_TRUE(sgo || !hasHad) failed: file /builds/worker/checkouts/gecko/dom/base/nsContentUtils.cpp:5037
[task 2020-10-23T23:03:06.991Z] 23:03:06     INFO -  (xpcshell/head.js) | test run_next_test 1 finished (2)
[task 2020-10-23T23:03:06.991Z] 23:03:06     INFO -  Unexpected exception NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIParserUtils.parseFragment]
[task 2020-10-23T23:03:06.991Z] 23:03:06     INFO -  textToHtmlDocumentFragment@resource:///modules/calendar/utils/calViewUtils.jsm:394:36
[task 2020-10-23T23:03:06.992Z] 23:03:06     INFO -  field@resource:///modules/calendar/ltnInvitationUtils.jsm:92:38
[task 2020-10-23T23:03:06.992Z] 23:03:06     INFO -  createInvitationOverlay@resource:///modules/calendar/ltnInvitationUtils.jsm:106:10
[task 2020-10-23T23:03:06.992Z] 23:03:06     INFO -  createInvitationOverlay_test@/builds/worker/workspace/build/tests/xpcshell/tests/comm/calendar/test/unit/test_ltninvitationutils.js:419:30
[task 2020-10-23T23:03:06.993Z] 23:03:06     INFO -  _run_next_test/<@/builds/worker/workspace/build/tests/xpcshell/head.js:1630:22
[task 2020-10-23T23:03:06.993Z] 23:03:06     INFO -  _run_next_test@/builds/worker/workspace/build/tests/xpcshell/head.js:1630:38
[task 2020-10-23T23:03:06.993Z] 23:03:06     INFO -  run@/builds/worker/workspace/build/tests/xpcshell/head.js:777:9
[task 2020-10-23T23:03:06.993Z] 23:03:06     INFO -  _do_main@/builds/worker/workspace/build/tests/xpcshell/head.js:250:6
[task 2020-10-23T23:03:06.993Z] 23:03:06     INFO -  _execute_test@/builds/worker/workspace/build/tests/xpcshell/head.js:577:5
[task 2020-10-23T23:03:06.993Z] 23:03:06     INFO -  @-e:1:1
[task 2020-10-23T23:03:06.993Z] 23:03:06     INFO -  exiting test

Caused by something from https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=1980f87855fc557e7ef6bb24b66c4d2df5606afa - bug 1666300 would be my first guess```

Comment 1

[Magnus Melin [:mkmelin]]

The parsing failure starts off from https://searchfox.org/comm-central/rev/ccdaa27822e254ca10d9c576c9a3723e141a05ae/calendar/test/unit/test_ltninvitationutils.js#419

Started by the https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=1980f87855fc557e7ef6bb24b66c4d2df5606afa merge - bug 1666300 is the main suspect. Henri, I don't have access to that bug. Anything obvious we need to adapt to?

Comment 2

[Intermittent Failures Robot]

29 failures in 4230 pushes (0.007 failures/push) were associated with this bug in the last 7 days.

This is the #45 most frequent failure this week.

Repository breakdown:

• comm-central: 29

Platform breakdown:

• linux64: 3
• macosx1014-64: 1
• windows7-32: 1
• windows10-64: 2
• windows7-32-shippable: 4
• macosx1014-64-shippable: 8
• windows10-64-shippable: 4
• linux64-shippable: 6

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1673164&startday=2020-10-19&endday=2020-10-25&tree=all

Comment 3

[Henri Sivonen (:hsivonen)]

(In reply to Magnus Melin [:mkmelin] from comment #1)

Anything obvious we need to adapt to?

Not obvious to me from the test case, but the thing to look for is if you try to set innerHTML on a document that was in an iframe but the iframe has been destroyed by the time you try to set innerHTML.

Comment 4

[Magnus Melin [:mkmelin]]

Tracked it down a bit.

/// Case A - works

var pu = Cc["@mozilla.org/parserutils;1"].createInstance(Ci.nsIParserUtils);
var flags = pu.SanitizerLogRemovals | pu.SanitizerDropForms | pu.SanitizerDropMedia;
var uri = Services.io.newURI("chrome://lightning/content/lightning-invitation.xhtml");
var context = document.createElement("div");
pu.parseFragment("some html", flags, false, uri, context);

/// Case B - fails. Only doc is different.

// parseFile is simply a sync XMLHttpRequest read: <https://searchfox.org/comm-central/rev/8d9e45511a64e83e3932983d036eca2a353a9261/calendar/base/modules/utils/calXMLUtils.jsm#144>
var doc = cal.xml.parseFile("chrome://lightning/content/lightning-invitation.xhtml");

var pu = Cc["@mozilla.org/parserutils;1"].createInstance(Ci.nsIParserUtils);
var flags = pu.SanitizerLogRemovals | pu.SanitizerDropForms | pu.SanitizerDropMedia;
var uri = Services.io.newURI("chrome://lightning/content/lightning-invitation.xhtml");
var context = doc.createElement("div");
pu.parseFragment("some html", flags, false, uri, context);

Case B doesn't work, will fail at !inert https://hg.mozilla.org/mozilla-central/rev/9143f95d5ab3#l2.27

So are we doing something wrong, or is it a bug?

Comment 5

[Intermittent Failures Robot]

31 failures in 865 pushes (0.036 failures/push) were associated with this bug yesterday.

Repository breakdown:

• comm-central: 31

Platform breakdown:

• linux64: 5
• windows10-64: 6
• windows7-32: 5
• linux64-shippable: 2
• windows7-32-shippable: 3
• windows10-64-shippable: 2
• macosx1014-64: 4
• macosx1014-64-shippable: 4

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1673164&startday=2020-10-27&endday=2020-10-27&tree=all

Comment 6

[Magnus Melin [:mkmelin]]

The problem caught by the test is that event invites in emails are broken.

Henri, any input? It's the parserUtils.parseFragment call that fails.
I don't know what an inert document is, or how this all fits into the bug fixed.

Comment 7

[Henri Sivonen (:hsivonen)]

Can you check what makes CreateInertDocument return early?

I'm guessing this line:
https://searchfox.org/mozilla-central/rev/c409dd9235c133ab41eba635f906aa16e050c197/dom/base/nsContentUtils.cpp#5037

smaug, how essential is that early return?

Comment 8

[Magnus Melin [:mkmelin]]

That's indeed the spot that bails out.

Comment 9

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

Using a document which used to be connected to a global but isn't anymore is worrisome.
Does cal.xml.parseFile("chrome://lightning/content/lightning-invitation.xhtml"); somehow run in a context where the global js object has already died? Or does the global object perhaps die right after calling parseFile?

Null script handling object means that js wrappers for example may get created using somewhat random js global (whoever is the caller).
So one really should try to not use such documents in JS.

(in the old days not having scriptglobal lead to security issues)

Comment 10

[Magnus Melin [:mkmelin]]

You may be on to something wrt global js object (though I don't know how to check).
If I inline the small parseFile function and use it directly there seems to be no problem.

Comment 11

[Magnus Melin [:mkmelin]]

Correction: inlining works in the console. In the real code it doesn't help

function parseFileX(uri) {
  var req = new XMLHttpRequest();
  req.open("GET", uri, false);
  req.overrideMimeType("text/xml");
  req.send(null);
  return req.responseXML;
}

var docURI = "chrome://lightning/content/lightning-invitation.xhtml";
var parseFile = cal.xml.parseFile; // works in the console if i set to parseFileX
var doc = parseFile(docURI);
var pu = Cc["@mozilla.org/parserutils;1"].createInstance(Ci.nsIParserUtils);
var flags = pu.SanitizerLogRemovals | pu.SanitizerDropForms | pu.SanitizerDropMedia;
var uri = Services.io.newURI(docURI);
var context = doc.createElement("div");
pu.parseFragment("some html", flags, false, uri, context);

I'm out of clues atm how that could be different, other than some slight timing being off perhaps.

Comment 12

[Henri Sivonen (:hsivonen)]

In this case, parseFile returns a doc marked "loaded as data", so perhaps we should omit the throw-away "loaded as data" doc when the main one already has that bit set.

Comment 13

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1673164 - Avoid creating an inert doc when the context is already inert.

Comment 14

[Henri Sivonen (:hsivonen)]

Does the attached patch fix things?

Comment 15

[Magnus Melin [:mkmelin]]

It fixes the problem above, but now the input is no longer sanitized like it used to, at least before this bug 1667113. Our test is checking that and therefore fails.

Comment 16

[Wayne Mery (:wsmwk)]

FYI This is a blocker for beta, and is now impacting our ability to deliver and test critical fixes for other issues, including sec-* fixes needed for release 78 that firefox have already shipped. I know you are working hard at this, but we do need a solution quickly.

Comment 17

[Henri Sivonen (:hsivonen)]

I will need to sort out testing for the patch before requesting for review.

Comment 18

[Intermittent Failures Robot]

18 failures in 711 pushes (0.025 failures/push) were associated with this bug yesterday.

Repository breakdown:

• comm-central: 18

Platform breakdown:

• linux64: 4
• linux64-shippable: 5
• macosx1014-64-shippable: 6
• macosx1014-64: 3

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1673164&startday=2020-10-30&endday=2020-10-30&tree=all

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d51fba42ee25
Avoid creating an inert doc when the context is already inert. r=smaug

Comment 20

[Dorel Luca [:dluca]]

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

Comment 21

[Intermittent Failures Robot]

118 failures in 4275 pushes (0.028 failures/push) were associated with this bug in the last 7 days.

This is the #7 most frequent failure this week.

** This failure happened more than 75 times this week! Resolving this bug is a very high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 1 week, the affected test(s) may be disabled. **

Repository breakdown:

• comm-central: 118

Platform breakdown:

• windows10-64-shippable: 13
• linux64-shippable: 22
• windows7-32-shippable: 14
• macosx1014-64-shippable: 27
• linux64: 14
• windows10-64: 10
• windows7-32: 7
• macosx1014-64: 11

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1673164&startday=2020-10-26&endday=2020-11-01&tree=all

Comment 22

[Henri Sivonen (:hsivonen)]

Comment on attachment 9184232 [details]
Bug 1673164 - Avoid creating an inert doc when the context is already inert.

[Approval Request Comment]
Regression caused by (bug #): Bug 1667113
User impact if declined: In Thunderbird, can't handle calendar invitations. In Firefox, the impact is unclear, but there's a chance of misbehavior if there's impacted chrome code with lacking test coverage.
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): Should be low risk.

[Approval Request Comment]
Regression caused by (bug #): Bug 1667113
User impact if declined: In Thunderbird, can't handle calendar invitations. In Firefox, the impact is unclear, but there's a chance of misbehavior if there's impacted chrome code with lacking test coverage.
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): Should be low risk.

Comment 23

[Henri Sivonen (:hsivonen)]

Not sure how to get an m-c patch on the m-c uplift radar when the bug is filed on the Thunderbird side. In any case, this should be uplifted to the same branches that bug 1667113 was uplifted to.

Comment 24

[Henri Sivonen (:hsivonen)]

Moving to Core in order to be able to set m-c-relevant request flags.

Comment 25

[Henri Sivonen (:hsivonen)]

Comment on attachment 9184232 [details]
Bug 1673164 - Avoid creating an inert doc when the context is already inert.

Beta/Release Uplift Approval Request

• User impact if declined: In Thunderbird, can't handle calendar invitations. In Firefox, the impact is unclear, but there's a chance of misbehavior if there's impacted chrome code with lacking test coverage.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This reverts to old flow when the context is already inert.
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This fixes a regression from bug 1667113, so this should be uplifted to the same branches as bug 1667113.
• User impact if declined: In Thunderbird, can't handle calendar invitations. In Firefox, the impact is unclear, but there's a chance of misbehavior if there's impacted chrome code with lacking test coverage.
• Fix Landed on Version: 84
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This reverts to old flow when the context is already inert.
• String or UUID changes made by this patch: None

Comment 26

[Pascal Chevrel:pascalc]

Comment on attachment 9184232 [details]
Bug 1673164 - Avoid creating an inert doc when the context is already inert.

Fix for a 83/84 regression, evaluated as low risk as we revert to our previous behaviour, approved for 83 beta 8, and esr78 thanks.

Comment 27

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0691f163eb41

Comment 28

[Ryan VanderMeulen [:RyanVM]]

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