1477253 - AV1 decoder is turned on by default !

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Jean-Yves Avenard [:jya]]

This is a regression from bug 1448222
https://searchfox.org/mozilla-central/source/modules/libpref/init/StaticPrefList.h#867

That's pretty bad...

AV1 is turned on by default, including in 61 which is rather ancient code.

Comment 1

[Nils Ohlmeier [:drno]]

Given that any code before the landing of the AV1 release in 63 is based on code from last September which has a lot of known sec issues I'm setting sec-high.

Comment 2

[Nils Ohlmeier [:drno]]

It looks like the MOZ_AV1 define around the AV1 prevented this from going to this all way into Release with 61 (I just tested it with 61 and 62).

Comment 3

[Nils Ohlmeier [:drno]]

Attachment: Turn off AV1 by default

Comment 4

[Jean-Yves Avenard [:jya]]

do we need security approval for this?

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8993684 [details] [diff] [review]
Turn off AV1 by default

[Security approval request comment]
How easily could an exploit be constructed based on the patch? need to hack around AV1, something we had enabled for over almost a year

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? hard to say...

Which older supported branches are affected by this flaw? all nightly since 61

If not all supported branches, which bug introduced the flaw?  1448222

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? no need

How likely is this patch to cause regressions; how much testing does it need? none

Comment 6

[Jean-Yves Avenard [:jya]]

ok, so as per https://wiki.mozilla.org/Security/Bug_Approval_Process

we fit under the:
     A specific regressing check-in has been identified
    The developer can (and has) marked the status flags for ESR, Beta, and Aurora as "unaffected"
    We have not shipped this vulnerability in anything other than a nightly build

so we can land...

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 8993684 [details] [diff] [review]
Turn off AV1 by default

Not needed as you noted, but sec-approval=dveditz anyway since my query already picked this up

Comment 8

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/da3f76df8820bd2144d34c8c8cc98ae6d73a1664
https://hg.mozilla.org/mozilla-central/rev/da3f76df8820

Comment 9

[Nicholas Nethercote [inactive]]

Apologies for the error :(

I just went back and double-checked all the pref values in bug 1448222's patch. As well as media.av1.enabled being incorrectly changed from false to true, I found one other error, involving media.memory_caches_combined_limit_kb:

- Before the patch it was always 524288.

- After the patch it is 524288, except on Android where it is 32768.

(I think this error must have occurred because the nearby media.cache_size pref had a value of 32768 on Android both before and after the patch.)

drno, should the value of media.memory_caches_combined_limit_kb be changed to 524288 on Android? If so, I'm happy to do that in a separate bug. (I would have asked jya, but he's on PTO.)

Comment 10

[Nils Ohlmeier [:drno]]

(In reply to Nicholas Nethercote [:njn] from comment #9)
> drno, should the value of media.memory_caches_combined_limit_kb be changed
> to 524288 on Android? If so, I'm happy to do that in a separate bug. (I
> would have asked jya, but he's on PTO.)

Thanks for double checking. Yes sounds like a good idea to restore the original value on that one as well in a new bug.

Comment 11

[Nicholas Nethercote [inactive]]

I filed bug 1479631 to fix media.memory_caches_combined_limit_kb.