1205456 - Release-mode crashes when serializing a principal with unknown app id

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Bobby Holley (:bholley)]

We fixed this in bug 1182610 for explicitly creating the origin, but origin stringification also happens implicitly during binary serialization. We need to handle that case too.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Check for UNKNOWN_APP_ID when serializing principals. v1

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e0f3104478f

Comment 3

[Bobby Holley (:bholley)]

Comment on attachment 8662079 [details] [diff] [review]
Check for UNKNOWN_APP_ID when serializing principals. v1

We should uplift this to 42. I'm assuming I won't get it uplifted to aurora before the branch, so flagging for beta.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1182610#c9

Approval Request Comment
[Feature/regressing bug #]: bug 1165162
[User impact if declined]: crashes
[Describe test coverage new/current, TreeHerder]: tests with patch
[Risks and why]: very low risk
[String/UUID change made/needed]: None

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0e0f3104478f

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8662079 [details] [diff] [review]
Check for UNKNOWN_APP_ID when serializing principals. v1

Fix a crash, taking it.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c9a4c0cc5881

Comment 7

[Bobby Holley (:bholley)]

Comment on attachment 8662079 [details] [diff] [review]
Check for UNKNOWN_APP_ID when serializing principals. v1

Nominating for uplift per Ritu's request.

Approval Request Comment
[Feature/regressing bug #]: bug 1165162
[User impact if declined]: Crashes
[Describe test coverage new/current, TreeHerder]: CI
[Risks and why]: Very low risk. Just propagating a rare failure condition instead of crashing.
[String/UUID change made/needed]: None

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

I was told by dmajor and philipp that I should consider taking this fix for 41 dot release. Bholley mentioned that this is an extremely safe fix. Also because the crash in bug 1208008 was reproducible, Philipp has verified that the crash has gone away on Nightly44 ever since the fix landed.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8662079 [details] [diff] [review]
Check for UNKNOWN_APP_ID when serializing principals. v1

The fix looks safe. Please see comment 8 for my reasoning for taking this as a ride-along, this is a verified and safe patch to take.

Comment 10

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

This didn't apply totally cleanly while uplifting, so hopefully this works...

https://hg.mozilla.org/releases/mozilla-release/rev/f662edb63d39

Comment 11

[:Paolo Amadini]

I've attached a test case for the Mozilla Archive Format add-on in bug 1208008.

I've seen the FIREFOX_41_0_1_RELEASE tag in the mozilla-release repository. Does this mean 42.0.1 is scheduled for release soon? I've received several reports of the crash over the week-end and I'd like to reply with a target date, or recommend users to upgrade to Firefox Beta in the meantime.

Comment 12

[Mike Kaply [:mkaply]]

Did this make it into 41.0.1? It's not in the release notes:

https://www.mozilla.org/en-US/firefox/41.0.1/releasenotes/

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Mike Kaply [:mkaply] from comment #12)
> Did this make it into 41.0.1? It's not in the release notes:
> 
> https://www.mozilla.org/en-US/firefox/41.0.1/releasenotes/

Mike, yes it was included in 41.0.1 dot release that went live late last week. I might have missed adding these to the release notes. Sorry about that! If you refresh the notes in 15 minutes or so from now, you should be able to see this one in there.

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Added this one to 41.0.1 release notes.

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Paolo, 41.0.1 was pushed live on Oct 2nd, 2015 noon PST. And I can confirm that this fix was included in it.

Comment 16

[:Paolo Amadini]

This was also reported as fixed by users of the Mozilla Archive Format add-on. Thanks!