crash in PrioEncoder when called with invalid public keys

RESOLVED FIXED in Firefox 64

Status

()

defect
P1
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: rhelmer, Assigned: rhelmer)

Tracking

unspecified
Firefox 64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

9 months ago
When one of the Prio public keys (prio.publicKeyA or prio.publicKeyB in preferences) are invalid, PrioEncoder crashes on the second invocation:

https://crash-stats.mozilla.com/report/index/8a88475d-685b-4e43-81c7-3823b0180917

Pretty sure that's because we're setting the singleton and calling Prio_init() here: https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/dom/prio/PrioEncoder.cpp#57

But we're not checking to see if the keys are valid until below: https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/dom/prio/PrioEncoder.cpp#67

So the first time PrioEncoder is called it throws as expected, but the next time it actually makes it all the way to the Encode() function and crashes.

This shouldn't be anything anyone will hit in the wild who doesn't play with these prefs, and also PrioEncoder is only used in Nightly, but we do eventually want to be able to change these via pref flip in the future and for QA purposes.

I found this while adding tests for bug 1485620, so I'll land that test and then add this and the fix for it.
Assignee

Updated

9 months ago
Assignee: nobody → rhelmer
Priority: -- → P1
Comment on attachment 9009798 [details]
Bug 1491737 - ensure that PrioEncoder handles bad public keys correctly r?hsivonen

Henri Sivonen (:hsivonen) has approved the revision.
Attachment #9009798 - Flags: review+

Comment 3

9 months ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32672615c473
ensure that PrioEncoder handles bad public keys correctly r=hsivonen
Assignee

Comment 4

9 months ago
Filed bug 1493584 as a followup to avoid further potential problems if any errors are thrown by `PrioEncoder.encode` during Prio initialization.

Comment 5

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/32672615c473
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.