1762614 - Update libopus

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement)

Description

[Tom Ritter [:tjr]]

Attachment: commits.txt

We last updated opus 4 years ago in Bug 1487049. I've attached the list of commits between our in-tree and HEAD.

In the commits list, there were (at least) six commits that referenced fixing a buffer or integer overflow; but I don't know if we are vulnerable to those specific issues, or if they are exploitable.

Comment 1

[Tom Ritter [:tjr]]

Attachment: commits.txt

Comment 2

[Tom Ritter [:tjr]]

Attachment: Bug 1762614: Update libopus to 2654707e86cc94413998976d179b2ab4a2aa3114 r?kinetik

Depends on D142711

Comment 3

[Tom Ritter [:tjr]]

Attachment: Bug 1762614: Remove unneccessary files r?kinetik

Depends on D142717

Comment 4

[Tom Ritter [:tjr]]

Bryce, could you identify someone to review this?

Comment 5

[Tom Ritter [:tjr]]

I've put in a try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b92d50730d0f7b19868ef3b920b8f2ea0b3f1723

I think the files I removed were leftover artifacts (they do not exist in the upstream repo) but the build should confirm that...

Comment 6

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

:kinetik, is this something you could help wrangle?

Comment 7

[Matthew Gregan [:kinetik]]

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #5)

I think the files I removed were leftover artifacts (they do not exist in the upstream repo) but the build should confirm that...

From a quick look, I think the Ne10 stuff is not used in our build. The x86/SSE removals are due to file renames, and appear to have been added correctly with the new names.

Patches look good - happy to review this when it's ready.

Comment 8

[Daniel Veditz [:dveditz]]

opus is being fuzzed as part of oss-fuzz, but that doesn't help us if we don't keep up with the fixes. It has not been separately fuzzed in Firefox since before oss-fuzz. It's possible some of the overflow bugs are regressions that wouldn't be in the copy of opus we have, but until we know that applies to all of them we should assume the worst.

Comment 9

[Tom Ritter [:tjr]]

Comment on attachment 9270483 [details]
Bug 1762614: Update libopus to 2654707e86cc94413998976d179b2ab4a2aa3114 r?kinetik

Approved to land

Comment 10

[Tom Ritter [:tjr]]

Comment on attachment 9270484 [details]
Bug 1762614: Remove unneccessary files r?kinetik

Approved to land

Comment 11

[Tom Ritter [:tjr]]

[Tracking Requested - why for this release]: We are assuming the worst here

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed:
https://hg.mozilla.org/integration/autoland/rev/26d845c1cc155e29fe06e0e011f8c192814d4122
https://hg.mozilla.org/integration/autoland/rev/f863c6888fe74e7705c991e2a257f0470318baba

Backed out for causing android arm build bustages
https://hg.mozilla.org/integration/autoland/rev/389511345c7cf93cd7136ce5d0279fae4ff28cb5

Push with bustage: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry&revision=f863c6888fe74e7705c991e2a257f0470318baba&selectedTaskRun=K03YkWglRne38XWwlMqi1g.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=373624334&repo=autoland

[task 2022-04-06T16:31:34.866Z] 16:31:34 ERROR - gmake[4]: *** No rule to make target 'celt_pitch_xcorr_arm-gnu.s', needed by 'celt_pitch_xcorr_arm-gnu.o'. Stop.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Update libopus to 2654707e86cc94413998976d179b2ab4a2aa3114 r=kinetik
https://hg.mozilla.org/integration/autoland/rev/394ca0b838e119778df65fe81a2485c4455f5c71
https://hg.mozilla.org/mozilla-central/rev/394ca0b838e1

Remove unneccessary files r=kinetik
https://hg.mozilla.org/integration/autoland/rev/a1f2c91e87c17b607e007d2cc5a6d2e7b3300c0a
https://hg.mozilla.org/mozilla-central/rev/a1f2c91e87c1

Comment 14

[Tom Ritter [:tjr]]

Comment on attachment 9270483 [details]
Bug 1762614: Update libopus to 2654707e86cc94413998976d179b2ab4a2aa3114 r?kinetik

Beta/Release Uplift Approval Request

• User impact if declined: We think that opus may have sec-high issues latent in it.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): We have tested this in Nightly but it's possible there are subtle failures or misbehaviors on uncommon platforms or videos we haven't noticed.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: We think that opus may have sec-high issues latent in it.
• User impact if declined: They would be vulnerable.
• Fix Landed on Version:
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): We have tested this in Nightly but it's possible there are subtle failures or misbehaviors on uncommon platforms or videos we haven't noticed.

Comment 15

[Dianna Smith [:diannaS]]

Comment on attachment 9270483 [details]
Bug 1762614: Update libopus to 2654707e86cc94413998976d179b2ab4a2aa3114 r?kinetik

Approved for 100.0b6

Comment 16

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d49e3767ca3c
https://hg.mozilla.org/releases/mozilla-beta/rev/b1caa23f5be3

Comment 17

[Dianna Smith [:diannaS]]

Comment on attachment 9270483 [details]
Bug 1762614: Update libopus to 2654707e86cc94413998976d179b2ab4a2aa3114 r?kinetik

Approved for 91.9esr

Comment 18

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/18bcd9e661c5
https://hg.mozilla.org/releases/mozilla-esr91/rev/e78c3b83a1e9