1962421 - (CVE-2025-5283) UAF crash in libvpx encoder

Status: RESOLVED FIXED

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

Description

[Randell Jesup [:jesup] (needinfo me)]

47 crashes with this stack in the last 6 months. All with 0xe5e5 addresses, so this is almost certainly a real bug in libvpx.

Crash report: https://crash-stats.mozilla.org/report/index/17b7a381-979d-4724-9000-0e8340250401

Reason:

EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  mozglue.dll  arena_dalloc(void*, unsigned long long, arena_t*)  memory/build/mozjemalloc.cpp:4721
0  mozglue.dll  BaseAllocator::free(void*)  memory/build/mozjemalloc.cpp:5671
0  mozglue.dll  MozJemalloc::free(void*)  memory/build/malloc_decls.h:63
0  mozglue.dll  PageFree(mozilla::Maybe<unsigned long long> const&, void*)  memory/build/PHC.cpp:1792
0  mozglue.dll  MozJemallocPHC::free(void*)  memory/build/PHC.cpp:1796
0  mozglue.dll  ReplaceMalloc::free(void*)  memory/build/malloc_decls.h:63
0  mozglue.dll  je_free(void*)  memory/build/malloc_decls.h:63
1  gkcodecs.dll  vpx_codec_enc_init_multi_ver(vpx_codec_ctx*, vpx_codec_iface const*, vpx_code...  media/libvpx/libvpx/vpx/src/vpx_encoder.c:136
2  xul.dll  webrtc::(anonymous namespace)::LibvpxFacade::codec_enc_init_multi(vpx_codec_c...  third_party/libwebrtc/modules/video_coding/codecs/interface/libvpx_interface.cc:68
3  xul.dll  webrtc::LibvpxVp8Encoder::InitAndSetControlSettings()  third_party/libwebrtc/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc:864

Comment 1

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

I have had a look going back and forth on this, and I think I found the root cause.

The poisoned pointer is the LOWER_RES_FRAME_INFO::mb_info here. It was just successfully allocated on the crashing thread, but upon a free due to another error we crash on an address with deallocation poison.

There are two theoretical paths to free mb_info. Either a local free as a revert before returning an error from init, but that is the free in the report; or a free of the config member holding mb_info as part of destroy, but then it must have been stored on the config and then exhibited an error when setting the compressor. Even if this is theoretically possible, I don't see any races that could trigger that path before init has finished. All paths for manual calls to destroy are on the encoder queue, which is also where the crash is. There's also one path to destroy the vp8 encoder by destroying the owning VideoStreamEncoder, but it also owns the encoder queue, which would get destroyed first due to declaration order. Destroying the encoder queue is a blocking call so we couldn't move on to destroying the vp8 encoder unless the encoder queue had been drained first, but it's on the encoder queue that we crash.

---

Looking at a higher level in crash-stats, I see related crashes that are for the same stack but not on a poisoned address. It suggests these two patterns may be two sides of a race. There's also bug 1871551 linked from a related signature, but I don't have access.

---

Hang on, looking again at the lower level code in libvpx where this crashes, there's an interesting for loop that certainly complicates the logic.
mem_loc and mem_loc->mb_info allocation happens outside this for loop, but cleanup on error happens inside. Let's assume we loop twice, with the first round being successful, i.e. pointers to this allocated memory was set succcessfully on an encoder context (impl 1->2 sets ctx->priv->oxcf->mr_low_res_mode_info = mem_loc). On the second round of the loop, let's assume init fails. Then the previously initialized encoder gets destroyed, but only if its encoder id matches ctx->oxcf.mr_total_resolutions - 1, which it does for i==0!

Note the offending free here is from bug 1426988 which was upstreamed to libvpx in https://issues.webmproject.org/issues/42330499.

There was a similar issue to this one reported in https://issues.webmproject.org/issues/42330651, but the fix didn't cover mr_low_res_mode_info being set from within the for loop.

Marking bug 1426988 as the regressor just to have some tracking. It did narrow down a wider heap-buffer-overflow to this UAF.

Comment 2

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Filed upstream as https://issues.webmproject.org/issues/413411335.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1426988

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criteria:

• Top 20 desktop browser crashes on release (startup)
• Top 10 desktop browser crashes on nightly

For more information, please visit BugBot documentation.

Comment 5

[Wan-Teh Chang]

We checked in a fix for this bug in libvpx yesterday:
https://chromium.googlesource.com/webm/libvpx/+/1c758781c428c0e895645b95b8ff1512b6bdcecb

We came up with a state where this bug occurs. It is what Andreas described in comment#1, but without any races, and init fails in that way in the first round of the loop.

The most likely reason libvpx enters that state is that vpx_malloc(), vpx_calloc(), or vpx_memalign() returns NULL. The second most likely reason is that pthread_create() returns an error.

We are not sure if the first reason is possible in Firefox. Could you answer these two questions?

1. In Firefox, if the malloc() function runs out of memory, does it return NULL, or does it abort the process (for security reasons)?
2. In Firefox, does the build system define the macro VPX_MAX_ALLOCABLE_MEMORY when compiling libvpx?

Thanks!

Comment 6

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

(In reply to Wan-Teh Chang from comment #5)

We checked in a fix for this bug in libvpx yesterday:
https://chromium.googlesource.com/webm/libvpx/+/1c758781c428c0e895645b95b8ff1512b6bdcecb

We came up with a state where this bug occurs. It is what Andreas described in comment#1, but without any races, and init fails in that way in the first round of the loop.

The most likely reason libvpx enters that state is that vpx_malloc(), vpx_calloc(), or vpx_memalign() returns NULL. The second most likely reason is that pthread_create() returns an error.

We are not sure if the first reason is possible in Firefox. Could you answer these two questions?

1. In Firefox, if the malloc() function runs out of memory, does it return NULL, or does it abort the process (for security reasons)?
2. In Firefox, does the build system define the macro VPX_MAX_ALLOCABLE_MEMORY when compiling libvpx?

Thanks!

Thank you. I can confirm our malloc returns nullptr when it cannot allocate the requested memory. VPX_MAX_ALLOCABLE_MEMORY is not defined through the build system.

Now we just need to sort bug 1957480 to get this fix pulled in. Chun-min, have you had a chance to get further there?

Comment 7

[Ryan VanderMeulen (PTO, back July 27)]

Can we cherry-pick the upstream fix for now since we'll want to uplift it around anyway?

Comment 8

[Wan-Teh Chang]

Andreas: Thanks for the reply. This could explain why this bug occurs more often in Firefox than in Chrome. In Chrome, malloc() aborts the process when it runs out of memory.

Ryan: It should be safe to cherry-pick the upstream fix. For extra assurance, you can ask James Zern or me to review your cherry-pick or uplift patches.

Comment 9

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

I'll wire up a cherry-pick meta patch for vendoring.

Comment 10

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Cherry-pick upstream libvpx commit 1c758781c4. r?ng! r?wtc

Comment 11

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Re-vendor libvpx. r?ng!

Comment 12

[Ryan VanderMeulen (PTO, back July 27)]

FYI, the cherry-pick patches apply cleanly to Beta and ESR128 but will need a bit of rebasing for ESR115.

Comment 13

[C.M.Chang[:chunmin]]

I was unable to reproduce it locally or get a pernosco record on try, but this issue seems to disappear with the latest update on try: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=aDiOtPMlSEm4lfQjpZ3Bow.0&revision=1c2dd635c00fa05ba3cc2e105e348d2efff06d1a

Comment 14

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Thanks Chun-Min. I'd like to land first because of the uplifts, and you should be able to revert D248120 right before your two vendoring patches, and carry on normally after that.

Comment 15

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Comment on attachment 9485828 [details]
Bug 1962421 - Cherry-pick upstream libvpx commit 1c758781c4. r?ng! r?wtc

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I don't see how. You could easily find a decent description of the problem in the upstream commit. Still though, to trigger this you need to be in a near-OOM situation, and then we're already likely to crash because of an actual OOM somewhere else.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all, yes status flags are correct
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: esr115 doesn't apply cleanly but it's merely a matter of offseting the line numbers in the patch
• How likely is this patch to cause regressions; how much testing does it need?: Fairly unlikely because it is tiny and has minimal scope. I say fairly because this is low-level code which can be tricky.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 16

[Tom Ritter [:tjr]]

Comment on attachment 9485828 [details]
Bug 1962421 - Cherry-pick upstream libvpx commit 1c758781c4. r?ng! r?wtc

Approved to land and uplift

Comment 17

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Cherry-pick upstream libvpx commit 1c758781c4. r?ng! r?wtc

Original Revision: https://phabricator.services.mozilla.com/D248120

Comment 18

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Re-vendor libvpx. r?ng!

Original Revision: https://phabricator.services.mozilla.com/D248121

Comment 19

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Cherry-pick upstream libvpx commit 1c758781c4. r?ng! r?wtc

Original Revision: https://phabricator.services.mozilla.com/D248120

Comment 20

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Re-vendor libvpx. r?ng!

Original Revision: https://phabricator.services.mozilla.com/D248121

Comment 21

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Cherry-pick upstream libvpx commit 1c758781c4. r?ng!,wtc

Original Revision: https://phabricator.services.mozilla.com/D248120

Comment 22

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

Attachment: Bug 1962421 - Re-vendor libvpx. r?ng!

Original Revision: https://phabricator.services.mozilla.com/D248121

Comment 23

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Chance of double-free when creating a multi-layer VP8 encoder for WebRTC
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: fairly low
• Explanation of risk level: Simple change. I put fairly because this is pretty low-level C code which can be tricky
• String changes made/needed: none
• Is Android affected?: yes

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ac1e19e5dec
Cherry-pick upstream libvpx commit 1c758781c4. r=wtc,ng
https://hg.mozilla.org/integration/autoland/rev/b79d0e18e1d3
Re-vendor libvpx. r=ng,wtc

Comment 25

[Phabricator Automation]

firefox-esr128 Uplift Approval Request

• User impact if declined: Chance of double-free when creating a multi-layer VP8 encoder for WebRTC
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: fairly low
• Explanation of risk level: Simple change. I put fairly because this is pretty low-level C code which can be tricky
• String changes made/needed: none
• Is Android affected?: yes

Comment 26

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined: Chance of double-free when creating a multi-layer VP8 encoder for WebRTC
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: fairly low
• Explanation of risk level: Simple change. I put fairly because this is pretty low-level C code which can be tricky
• String changes made/needed: none
• Is Android affected?: yes

Comment 27

[Andreas Pehrson [:pehrsons] (PTO; back in Aug)]

See this crash-stats link for a way to track the number of crashes for this issue in the wild. The linked signature is too broad.

Comment 28

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

https://hg.mozilla.org/mozilla-central/rev/7ac1e19e5dec
https://hg.mozilla.org/mozilla-central/rev/b79d0e18e1d3

Comment 29

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/7d81d04d4823
https://hg.mozilla.org/releases/mozilla-beta/rev/dbfa03eba1e4

Comment 30

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/ac20f59bcf07
https://hg.mozilla.org/releases/mozilla-esr128/rev/69f53ce5ef0b

Comment 31

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/45d4ed0d9091
https://hg.mozilla.org/releases/mozilla-esr115/rev/f37bab5c17b2

Comment 32

[Jesse Schwartzentruber (:truber)]

Attachment: advisory.txt