1810790 - libprio could potentially allow a malicious client to corrupt the statistics that the system outputs

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Robert Helmer [:rhelmer]]

I received this report and patch from Henry Corrigan-Gibbs via email:

While looking back at the code for the libprio C library,
I noticed a security-relevant logic bug in the servers'
validation logic.

In particular, the bug could potentially allow a malicious
client to corrupt the statistics that the system outputs. So,
it's security-relevant, but is probably low impact -- it just
affects the correctness of the statistics and doesn't involve
memory errors or anything like that.

As far as I can tell, the bug does not affect libprio-rs.

The current version of Prio is based on the pilot C library https://github.com/mozilla/libprio which is vendored in mozilla-central (we haven't switched to the Rust version yet https://github.com/divviup/libprio-rs).

I'm not sure if we're actively using the C version of libprio in Firefox, we should consider removing it and moving to libprio-rs instead if there are plans to use it in the future.

Otherwise, we should probably ship the fix in Firefox before disclosing it in GitHub (it's also used by backend tooling, where this patch probably isn't relevant).

Comment 1

[Robert Helmer [:rhelmer]]

Do you know of any plans to use the C version of libprio? How would you feel about removing it?

Comment 2

[Chris H-C :chutten]

To my knowledge any existing use of the C library can be discontinued as current efforts aren't engaging with it (though content blocking may need to be asked if they're receiving and using the data from Origin Telemetry). Simon, is this correct?

Comment 3

[Simon Friedberger (:simonf)]

Yes, we're now already using the Rust version libprio-rs. We should try to remove the C library from mozilla-central, I think.

Comment 4

[Henry Corrigan-Gibbs [:henrycg]]

Makes sense to me!

Rob, should we add a note to the mozilla/libprio repository saying that we're no longer patching security bugs for it?

Comment 5

[Simon Friedberger (:simonf)]

Attachment: Bug 1810790 - Remove Origin Telemetry. r=chutten

Comment 6

[Robert Helmer [:rhelmer]]

(In reply to Henry Corrigan-Gibbs [:henrycg] from comment #4)

Makes sense to me!

Rob, should we add a note to the mozilla/libprio repository saying that we're no longer patching security bugs for it?

Yes I will do this and also archive it.

Comment 7

[Cristian Tuns]

Backed out for causing multiple failures

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | telemetry/marionette/tests/client/test_main_tab_scalars.py TestMainTabScalars.test_main_tab_scalars | AssertionError: Error waiting for ping: Timed out after 60.1 seconds

Xpcshell failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=1155668&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=74a3bf4410ea3cdc16fda4100e29d5398d8c3e15&searchStr=xpcshell&selectedTaskRun=WwunEzHqTDi3aRIDBOZgCA.0

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/30517fbc6b05
Remove Origin Telemetry. r=chutten,timhuang

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aefea186c9bc
Remove Origin Telemetry. r=chutten,timhuang

Comment 10

[Cristina Horotan [:chorotan]]

Backed out changeset aefea186c9bc (bug 1810790) for causing mochitest failures on performance/browser_preferences_usage.js

Backout: https://hg.mozilla.org/integration/autoland/rev/ba5f6662ca8058d3e646c042c5bbaa8b0ef027ca

Failure push: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=aefea186c9bc7585985a04d7c566400d0f85c9d9&selectedTaskRun=TTsOaO1cQyqIm682TpMPyg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=404340006&repo=autoland&lineNumber=3887

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/915d96f055b1
Remove Origin Telemetry. r=chutten,timhuang

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/915d96f055b1