2016358 - (CVE-2026-2778) StructuredCloneBlob OOB Heap Read via Crafted blobCount

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Sajeeb Lohani]

Compromised content process -> parent process via ContentParent::RecvAsyncMessage.

StructuredCloneBlob::Holder::ReadStructuredCloneInternal (StructuredCloneBlob.cpp:176) reads blobOffset and blobCount from the attacker-controlled StructuredClone buffer, then calls BlobImpls().AppendElements(&aHolder->BlobImpls()[blobOffset], blobCount). The AppendElements(const T*, size_type) overload takes a raw pointer and copies blobCount elements via std::uninitialized_copy with no source bounds checking. No release-build validation exists; the #ifdef FUZZING guard only checks blobOffset, not blobOffset + blobCount.

SCTAG_DOM_STRUCTURED_CLONE_HOLDER (StructuredCloneHolder.cpp:1121) has no CloneScope() == SameProcess guard, unlike SCTAG_DOM_IMAGEBITMAP at line 1105. So it's reachable from cross-process (IPC) scope.

The Bug

StructuredCloneBlob.cpp:165-177:

uint32_t blobOffset;
uint32_t blobCount;
if (!JS_ReadUint32Pair(aReader, &blobOffset, &blobCount)) {
    return false;
}
if (blobCount) {
#ifdef FUZZING
    if (blobOffset >= aHolder->BlobImpls().Length()) {  // incomplete even when present
        return false;
    }
#endif
    BlobImpls().AppendElements(&aHolder->BlobImpls()[blobOffset], blobCount);  // OOB
}

With 1 blob in the IPC message and blobCount=100: AppendElements reads 99 elements past the 1-element array. Each 8-byte OOB value is treated as a RefPtr<BlobImpl>, and the copy constructor calls AddRef() on non-null values, which dereferences garbage heap pointers.

ASAN Proof

SUMMARY: AddressSanitizer: heap-buffer-overflow (XUL:arm64+0x1b3128)
  in nsTArray_Impl::AppendElementsInternal

0x6020002278e0 is located 0 bytes after 16-byte region
  [0x6020002278d0,0x6020002278e0)

PoC

dom/base/test/gtest/TestStructuredCloneBlobOOB.cpp:

/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "gtest/gtest.h"

#include "nsTArray.h"
#include "nsISupportsImpl.h"
#include "mozilla/RefPtr.h"

namespace {

class SimpleRefCounted {
 public:
  NS_INLINE_DECL_REFCOUNTING(SimpleRefCounted)
  uint64_t mData = 0;

 private:
  ~SimpleRefCounted() = default;
};

}  // namespace

TEST(StructuredCloneBlobOOB, AppendElementsOOBRead)
{
  nsTArray<uint64_t> sourceArray;
  sourceArray.AppendElement(0xDEADBEEFCAFEBABEULL);
  ASSERT_EQ(sourceArray.Length(), 1u);

  uint32_t blobOffset = 0;
  uint32_t blobCount = 16;  // only 1 element, reading 16

  ASSERT_LT(blobOffset, sourceArray.Length());

  // ASAN: heap-buffer-overflow READ
  nsTArray<uint64_t> destArray;
  destArray.AppendElements(&sourceArray[blobOffset], blobCount);

  ASSERT_EQ(destArray[0], 0xDEADBEEFCAFEBABEULL);
  ASSERT_EQ(destArray.Length(), (uint32_t)blobCount);
}

TEST(StructuredCloneBlobOOB, DISABLED_RefPtrAddRefOnGarbage)
{
  nsTArray<RefPtr<SimpleRefCounted>> sourceArray;
  sourceArray.AppendElement(MakeRefPtr<SimpleRefCounted>());
  ASSERT_EQ(sourceArray.Length(), 1u);

  uint32_t blobOffset = 0;
  uint32_t blobCount = 4;

  // ASAN: heap-buffer-overflow READ (before the AddRef crash)
  nsTArray<RefPtr<SimpleRefCounted>> destArray;
  destArray.AppendElements(&sourceArray[blobOffset], blobCount);
}

TEST(StructuredCloneBlobOOB, ExactCountNoOverflow)
{
  nsTArray<uint64_t> sourceArray;
  sourceArray.AppendElement(0x1111111111111111ULL);
  sourceArray.AppendElement(0x2222222222222222ULL);
  sourceArray.AppendElement(0x3333333333333333ULL);

  uint32_t blobOffset = 1;
  uint32_t blobCount = 2;

  nsTArray<uint64_t> destArray;
  destArray.AppendElements(&sourceArray[blobOffset], blobCount);

  ASSERT_EQ(destArray.Length(), 2u);
  ASSERT_EQ(destArray[0], 0x2222222222222222ULL);
  ASSERT_EQ(destArray[1], 0x3333333333333333ULL);
}

Run: ./mach gtest "StructuredCloneBlobOOB.AppendElementsOOBRead"

Fix

Add bounds validation:

if (blobCount) {
    if (blobOffset >= aHolder->BlobImpls().Length() ||
        blobCount > aHolder->BlobImpls().Length() - blobOffset) {
        return false;
    }
    BlobImpls().AppendElements(&aHolder->BlobImpls()[blobOffset], blobCount);
}

Also add a CloneScope() == SameProcess guard for SCTAG_DOM_STRUCTURED_CLONE_HOLDER in StructuredCloneHolder.cpp:1121.

Comment 1

[Andrew McCreight [:mccr8]]

How is this an out of bounds read? AppendElements adds new elements, it doesn't read anything.

Also aHolder->BlobImpls() returns an nsTArray, so indexing at blobOffset will crash safely and not go out-of-bounds.

Comment 2

[Andrew McCreight [:mccr8]]

Well, I can reproduce an ASan crash locally so I'll take a look some more at it.

Comment 3

[Andrew McCreight [:mccr8]]

I talked over this with Nika and we both this this is a valid issue. Sorry for my initial skepticism, but this seemed like such an obvious problem, and we clearly have at least a bit of fuzzing coverage here due to the FUZZING but clearly we have some work to do there.

Comment 4

[Andrew McCreight [:mccr8]]

I looked up other places that do BlobImpls()[ and the others are just looking at a single element and not doing this weird raw pointer span thing.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: (secure)

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 9544722 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Probably pretty obvious, although maybe anybody who has been paying attention could have already found this.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• 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
• 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?: Shouldn't be a problem, as this code hasn't changed much. Note that this patch requires bug 2016498, or it'll just end up as another sec bug.
• How likely is this patch to cause regressions; how much testing does it need?: This should only cause problems in the cases where we're out of bounds anyways.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 9544722 [details]
(secure)

sec-approval+

Comment 8

[Andrew McCreight [:mccr8]]

If tomorrow is really the last day for beta, then maybe I should wait for the next release cycle, come to think of it, as I really want to make sure bug 2016498 has stuck and can be uplifted before landing this.

Comment 9

[Andrew McCreight [:mccr8]]

I realized I didn't post an actual explanation of what is going on here, which I should do because it isn't obvious and it took me a bit of staring to figure out.

The problematic code is BlobImpls().AppendElements(&aHolder->BlobImpls()[blobOffset], blobCount). This involves 2 nsTArrays, but the type of the elements doesn't matter so let's pretend they are nsTArray<T>.
I'll make some definitions so the code is shorter

• array1 is aHolder->BlobImpls() and array2 is BlobImpls()
• x is blobOffset and y is blobCount.

The original code is then array2.AppendElements(&array1[x], y). Thing go awry with the &. Breaking it down from the inside out:

1. array1[x] is a reference to the xth element of the nsTArray array1, with type T&.
2. &array1[x] is a pointer to the xth element of the nsTArray array1, with type T*.

The overload of AppendElements we are calling takes two arguments: a C-style array pointer, and the length of that array (in this case y). It then copies all y of those elements of the array onto the nsTArray (in this case array2).

The issue is that we have done nothing to ensure that the length of array2 is at least x + y. array1[x] is safe because array1 is an nsTArray and we do runtime bounds checking, but by the time we're dealing with the x + y we've dropped the nsTArray and only have a raw pointer.

x, y, and array1 are all controlled by the sender of the message, so it is trivial for the sender to make x + y larger than the length of array1. If the attacker manages to do heap grooming and control memory located after array1, then it can put whatever it wants into array2, which will be used later by the parent process.

What my patch does is change the above code to array2.AppendElements(Span(array1).Subspan(x, y)). A mozilla::Span is a C-style array plus a length, and access is bounds checked.

1. Span(array1) creates a span out of the entire nsTArray array1, which knows its length.
2. Span(array1).Subspan(x, y) then creates a subspan of length y starting from element x. Span does the appropriate runtime checks to ensure this is valid. Well, it is supposed to at least (see bug 2016498).

Anyways, as you can see the basic idea is that we never drop into a raw C-style array that can't be checked.

Comment 10

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Very old sec-high bug that could be used as a sandbox escape. Kind of an obvious problem in hindsight so it would be nice to get it fixed quickly despite how old it is.
• 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: This adds a new check that should only fail if we are in a bad state, so we probably prefer a crash. Note that this depends on bug 2016498, which is another sec-high tweak to a check.
• String changes made/needed: none
• Is Android affected?: yes

Comment 11

[Andrew McCreight [:mccr8]]

Attachment: (secure)

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

Comment 12

[Andrew McCreight [:mccr8]]

Attachment: (secure)

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

Comment 13

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: Very old sec-high bug that could be used as a sandbox escape. Kind of an obvious problem in hindsight so it would be nice to get it fixed quickly despite how old it is.
• 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: This adds a new check that should only fail if we are in a bad state, so we probably prefer a crash. Note that this depends on bug 2016498, which is another sec-high tweak to a check.
• String changes made/needed: none
• Is Android affected?: yes

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/01237783a03f
https://hg.mozilla.org/integration/autoland/rev/1479f78b642c
Use Span in ReadStructuredCloneInternal. r=nika

Comment 15

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined: Very old sec-high bug that could be used as a sandbox escape. Kind of an obvious problem in hindsight so it would be nice to get it fixed quickly despite how old it is.
• 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: This adds a new check that should only fail if we are in a bad state, so we probably prefer a crash. Note that this depends on bug 2016498, which is another sec-high tweak to a check.
• String changes made/needed: none
• Is Android affected?: yes

Comment 16

[Andrew McCreight [:mccr8]]

Attachment: (secure)

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

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg-edge.mozilla.org/mozilla-central/rev/1479f78b642c

Comment 18

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/6045adc7929d
https://hg.mozilla.org/releases/mozilla-beta/rev/d563aecb588e

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/a58215c1b79f

Comment 20

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/6b01cb17d47d