Closed Bug 1491737 Opened 2 years ago Closed 2 years ago

crash in PrioEncoder when called with invalid public keys

Categories

(Firefox :: General, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(1 file)

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: nobody → rhelmer
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+
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32672615c473
ensure that PrioEncoder handles bad public keys correctly r=hsivonen
Filed bug 1493584 as a followup to avoid further potential problems if any errors are thrown by `PrioEncoder.encode` during Prio initialization.
https://hg.mozilla.org/mozilla-central/rev/32672615c473
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.