2027287 - Integer overflow in [@ libyuv::ScaleYUVToARGBBilinearDown] leads to out-of-bounds heap read when src_height ≥ 32769

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bugmon [:jkratzer for issues]]

Summary

Signed integer overflow in a 16-bit fixed-point bounds computation. In gfx/ycbcr/scale_yuv_argb.cpp at line 620, (src_height - 1) << 16 overflows int when src_height ≥ 32769, producing a large negative max_y. The subsequent clamp if (y > max_y) { y = max_y; } then always fires for positive y, driving the row index yi to -32767. YUVBuferIter_MoveToForI420 multiplies this by the Y-plane stride to form a pointer ~128 KB before the Y buffer, and I422ToARGBRow_Any_AVX2 reads from it.

The root cause is that Mozilla's fork of libyuv (YUVToARGBScale) omits the src_height > 32768 bounds check that the upstream ARGBScale has.

Affected Code

File: gfx/ycbcr/scale_yuv_argb.cpp, line 620

const int max_y = (src_height - 1) << 16;   // <-- OVERFLOW: 32769 << 16 = 0x80010000 (signed)
const int max_yi = src_height - 1;
for (j = 0; j < dst_height; ++j) {
  yi = y >> 16;
  if (yi != lastyi) {
    if (y > max_y) {                        // positive y > huge-negative max_y: always true
      y = max_y;                            // y = -2147418112
      yi = y >> 16;                         // yi = -32767 (arithmetic shift)
    }
    if (yi != lastyi) {
      ...
      } else {
        iter.MoveTo(iter, yi);              // MoveTo(-32767)
        argb_cnv_rowptr = argb_cnv_row;
        argb_cnv_rowstride = kRowSize;
        YUVBuferIter_ConvertToARGBRow(iter, argb_cnv_rowptr);   // line 643: OOB READ

File: gfx/ycbcr/scale_yuv_argb.cpp, lines 185–193

static void YUVBuferIter_MoveToForI420(YUVBuferIter& iter, int y_index) {
  const int kYShift = 1;
  int uv_y_index = y_index >> kYShift;
  iter.y_index = y_index;
  iter.src_row_y = iter.src_y + y_index * iter.src_stride_y;   // src_y + (-32767 * stride)
  iter.src_row_u = iter.src_u + uv_y_index * iter.src_stride_u;
  iter.src_row_v = iter.src_v + uv_y_index * iter.src_stride_v;
}

File: gfx/ycbcr/scale_yuv_argb.cpp, lines 1109–1113 — missing bounds check

if (!src_y || !src_u || !src_v ||
    src_width == 0 || src_height == 0 ||          // <-- only checks zero, not > 32768
    !dst_argb || dst_width <= 0 || dst_height <= 0) {
  return -1;
}

Compare with upstream media/libyuv/libyuv/source/scale_argb.cc line 1204:

if (!src_argb || src_width == 0 || src_height == 0 || src_width > 32768 ||
    src_height > 32768 || !dst_argb || dst_width <= 0 || dst_height <= 0) {
  return -1;
}

The file header at line 46 explicitly documents that YUVToARGBScale() was derived from ARGBScale(), but the fork dropped the upper-bound check. The same overflow exists in ScaleYUVToARGBBilinearUp at line 692.

Correct Pattern

int YUVToARGBScale(const uint8_t* src_y, int src_stride_y,
                   const uint8_t* src_u, int src_stride_u,
                   const uint8_t* src_v, int src_stride_v,
                   uint32_t src_fourcc,
                   YUVColorSpace yuv_color_space,
                   int src_width, int src_height,
                   uint8_t* dst_argb, int dst_stride_argb,
                   int dst_width, int dst_height,
                   enum FilterMode filtering)
{
  if (!src_y || !src_u || !src_v ||
      src_width <= 0 || src_height <= 0 ||
      src_width > 32768 || src_height > 32768 ||    // <-- added upper bound
      !dst_argb || dst_width <= 0 || dst_height <= 0) {
    return -1;
  }
  ...
}

Exploit Chain

1. Caller invokes YUVToARGBScale with src_height = 32770, a small src_width (e.g. 4), a small destination (e.g. 4×100), and kFilterBilinear. The validation at line 1109 passes because src_height != 0.
2. ScaleYUVToARGB dispatches to ScaleYUVToARGBBilinearDown (line 1057) because filtering is enabled and dy ≥ 65536 (source much taller than destination).
3. At line 620, max_y = (32770 - 1) << 16 = 32769 << 16 = 0x80010000. As signed int32, this is -2,147,418,112.
4. The loop begins with y positive (computed from ScaleSlope). On the first iteration where yi != lastyi, the test y > max_y compares a positive value against -2,147,418,112 — always true.
5. y is set to -2,147,418,112; yi = y >> 16 arithmetic-shifts to 0xFFFF8001 = -32767.
6. iter.MoveTo(iter, -32767) → YUVBuferIter_MoveToForI420 computes iter.src_row_y = src_y + (-32767) * src_stride_y. With src_stride_y = 4, this is src_y - 131068.
7. YUVBuferIter_ConvertToARGBRow (line 643) calls I422ToARGBRow_Any_AVX2, which memcpy's from the wild src_row_y → SEGV or heap disclosure.

Steps to Reproduce

1. Apply the harness patch adding FuzzingFunctions.scaleYUVToARGB() (adds include paths for gfx/ycbcr and media/libyuv/libyuv/include, adds a WebIDL binding and C++ implementation that allocates correctly-sized I420 buffers and calls YUVToARGBScale).
2. Rebuild Firefox.
3. Set pref fuzzing.enabled = true.
4. Load:

   <!DOCTYPE html>
   <script>FuzzingFunctions.scaleYUVToARGB(4, 32770, 4, 100);</script>
   

Security Impact

Severity: Low (latent). The bug is currently unreachable from web content:

• The only designed caller, ConvertAndScaleFromYCbCrDescriptor, has zero call sites.
• The live wrapper ScaleYCbCrToRGB32 is only reached via gfx::ScaleYCbCrToRGB (YCbCrUtils.cpp:234) when aDestSize != mPictureRect.Size(), and current callers always pass matching sizes.

If a caller were introduced that let an attacker control src_height ≥ 32769 with non-trivial src_stride_y, the out-of-bounds read offset -32767 * stride could be aimed at adjacent heap objects. The leaked bytes would be transformed by the YUV→ARGB colour matrix, but since kYuvI601Constants is a known fixed matrix, the transform is invertible per-pixel and leaked bytes could be recovered from the output ARGB buffer — an information disclosure primitive.

Preconditions: A future caller that passes attacker-controlled src_height > 32768 with a matching Y-plane allocation and kFilterBilinear.

ASAN Report

==206055==ERROR: AddressSanitizer: SEGV on unknown address 0x78e59c0cd804 (pc 0x7ce5eb788974 bp 0x7ffce5e4d490 sp 0x7ffce5e4cc48 T0)
==206055==The signal is caused by a READ memory access.
    #0 0x7ce5eb788974 in __nss_database_lookup ??:?
    #1 0x55c8dbbc045b in __asan_memcpy _asan_rtl_:3
    #2 0x78e5ce33632e in I422ToARGBRow_Any_AVX2 /firefox/media/libyuv/libyuv/source/row_any.cc:404:1
    #3 0x78e5c198c1ab in libyuv::YUVBuferIter_ConvertToARGBRow(libyuv::YUVBuferIter&, unsigned char*) /firefox/gfx/ycbcr/scale_yuv_argb.cpp:205:3
    #4 0x78e5c198c1ab in libyuv::ScaleYUVToARGBBilinearDown(int, int, int, int, int, int, int, int, unsigned char const*, unsigned char const*, unsigned char const*, unsigned char*, int, int, int, int, libyuv::FilterMode, unsigned int, mozilla::gfx::YUVColorSpace) /firefox/gfx/ycbcr/scale_yuv_argb.cpp:643:13
    #5 0x78e5c198c1ab in libyuv::ScaleYUVToARGB(unsigned char const*, int, unsigned char const*, int, unsigned char const*, int, int, int, unsigned char*, int, int, int, libyuv::FilterMode, unsigned int, mozilla::gfx::YUVColorSpace) /firefox/gfx/ycbcr/scale_yuv_argb.cpp:1057:5
    #6 0x78e5c198c1ab in YUVToARGBScale /firefox/gfx/ycbcr/scale_yuv_argb.cpp:1117:3
    #7 0x78e5c3fe0ae1 in mozilla::dom::FuzzingFunctions::ScaleYUVToARGB(mozilla::dom::GlobalObject const&, int, int, int, int) /firefox/dom/base/FuzzingFunctions.cpp:113:3
    #8 0x78e5c60d0339 in mozilla::dom::FuzzingFunctions_Binding::scaleYUVToARGB(JSContext*, unsigned int, JS::Value*) /firefox/obj-x86_64-pc-linux-gnu/dom/bindings/./FuzzingFunctionsBinding.cpp:347:3
    #9 0x78e5cfd51eda in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /firefox/js/src/vm/Interpreter.cpp:490:13
    #10 0x78e5cfd51eda in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /firefox/js/src/vm/Interpreter.cpp:586:12
    #11 0x78e5cefebd3b in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICFallbackStub*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /firefox/js/src/jit/BaselineIC.cpp:1698:10
    #12 0x1315555cd823  (<unknown module>)

==206055==Register values:
rax = 0x00007ffce5e4d500  rbx = 0x0000000000000000  rcx = 0x000010001cbc1aa0  rdx = 0x0000000000000000
rdi = 0x00007ffce5e4d500  rsi = 0x000078e59c0cd804  rbp = 0x00007ffce5e4d490  rsp = 0x00007ffce5e4cc48
 r8 = 0x00000fff9cbc9aa0   r9 = 0x00007ffce5e4d503  r10 = 0x00000fff9cbc9aa0  r11 = 0x000010001cbc1aa0
r12 = 0x000010001cbc1aa0  r13 = 0xffffffffffffffc3  r14 = 0x000010001cbc1a9c  r15 = 0x0000000000000000
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libc.so.6+0x198974)
==206055==ABORTING

Note rsi = 0x78e59c0cd804 matches the faulting address — this is the memcpy source operand, i.e. the wild src_row_y.

Suggested Fix

Add the same upper-bound check that upstream libyuv's ARGBScale uses. Also change == 0 to <= 0 to reject negative dimensions:

--- a/gfx/ycbcr/scale_yuv_argb.cpp
+++ b/gfx/ycbcr/scale_yuv_argb.cpp
@@ -1107,7 +1107,8 @@ int YUVToARGBScale(const uint8_t* src_y, int src_stride_y,
                    enum FilterMode filtering)
 {
   if (!src_y || !src_u || !src_v ||
-      src_width == 0 || src_height == 0 ||
+      src_width <= 0 || src_height <= 0 ||
+      src_width > 32768 || src_height > 32768 ||
       !dst_argb || dst_width <= 0 || dst_height <= 0) {
     return -1;
   }

This matches the upstream contract: 16.16 fixed-point coordinates require dimensions ≤ 32768 so that (dim - 1) << 16 fits in a signed 32-bit integer.

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: test.html

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: source.patch

Comment 3

[Bugmon [:jkratzer for issues]]

Attachment: prefs.json

Comment 4

[Bugmon [:jkratzer for issues]]

Attachment: crash_stack.txt

Comment 5

[Bugmon [:jkratzer for issues]]

Attachment: prefs.js

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 8

[Christian Holler (:decoder)]

Prefs checked: fuzzing.enabled only gates the demonstration harness (FuzzingFunctions.scaleYUVToARGB, added via source patch) — the vulnerable code YUVToARGBScale itself ships ungated in release builds with real callers. Not an unsupported-config case; sec-low is appropriate for this latent defect.

This is an automated analysis result. If this result is incorrect please add a needinfo and feel free to correct the error.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0cb5a413b7f0
https://hg.mozilla.org/integration/autoland/rev/34d0e99c110d
r=gfx-reviewers,media-playback-reviewers,aosmond

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/34d0e99c110d

Comment 11

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:tnikkel, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox150 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 13

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: sec bug
• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: simple bounds check that upstream code has
• String changes made/needed?: none
• Is Android affected?: yes

Comment 14

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/0f1d3a7ee420
https://hg.mozilla.org/releases/mozilla-beta/rev/2b8d62e21c45