CVE-2026-7950
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifui/gfx/x/connection.h |
modified | |
TESTui/gfx/x/connection_unittest.cc |
modified | |
ifui/gfx/x/generated_protos/xproto.cc |
modified |
Files Changed
ui/gfx/x/connection.hui/gfx/x/connection_unittest.ccui/gfx/x/gen_xproto.pyui/gfx/x/generated_protos/xproto.cc
Patch
From 6c8a8e8cee9925e1a404b606b7e1f58d6b60e5e2 Mon Sep 17 00:00:00 2001
From: Tom Anderson <thomasanderson@chromium.org>
Date: Mon, 06 Apr 2026 11:50:12 -0700
Subject: [PATCH] x11: Validate GetProperty format and fix OOB access
R=thestig
Change-Id: I8426be3b2246375cc2cdff5fbaa2d9009d05020d
Fixed: 496259890
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7731969
Reviewed-by: Lei Zhang <thestig@chromium.org>
Auto-Submit: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1610384}
---
diff --git a/ui/gfx/x/connection.h b/ui/gfx/x/connection.h
index 766bd3f..7f2d9047 100644
--- a/ui/gfx/x/connection.h
+++ b/ui/gfx/x/connection.h
@@ -372,11 +372,14 @@
.long_length = static_cast<uint32_t>(
amount ? length : std::numeric_limits<lentype>::max())})
.Sync();
- if (!response || response->format / 8u != sizeof(T)) {
+ if (!response ||
+ (response->format != 8 && response->format != 16 &&
+ response->format != 32) ||
+ response->format / 8u != sizeof(T)) {
return false;
}
- size_t byte_len = response->value_len * response->format / 8u;
+ size_t byte_len = response->value_len * sizeof(T);
value->resize(response->value_len);
if (byte_len > 0u) {
UNSAFE_TODO(memcpy(value->data(), response->value->bytes(), byte_len));
diff --git a/ui/gfx/x/connection_unittest.cc b/ui/gfx/x/connection_unittest.cc
index 9116707..fc451c4f 100644
--- a/ui/gfx/x/connection_unittest.cc
+++ b/ui/gfx/x/connection_unittest.cc
@@ -123,4 +123,36 @@
connection.QueryTree(root).Sync();
}
+TEST(X11ConnectionTest, GetPropertyReplyValidation) {
+ // Simulate a malicious response with format 39.
+ // The reply length is in 4-byte units, starting from after the first 32
+ // bytes. A GetProperty reply has a fixed size of 32 bytes followed by the
+ // value.
+ std::vector<uint8_t> data(32, 0);
+ data[0] = 1; // response_type: Reply
+ data[1] = 39; // format: 39 (Invalid, should be 8, 16, or 32)
+ data[10] = 0; // length: 0
+ data[11] = 0;
+
+ ReadBuffer buf(x11::ThrowAwaySizeRefCountedMemory::From(std::move(data)));
+ auto reply = detail::ReadReply<GetPropertyReply>(&buf);
+ EXPECT_FALSE(reply);
+}
+
+TEST(X11ConnectionTest, GetPropertyReplyValid) {
+ // Simulate a valid response with format 32.
+ std::vector<uint8_t> data(32, 0);
+ data[0] = 1; // response_type: Reply
+ data[1] = 32; // format: 32
+ data[10] = 0; // length: 0
+ data[11] = 0;
+ data[16] = 0; // type: None
+ data[24] = 0; // value_len: 0
+
+ ReadBuffer buf(x11::ThrowAwaySizeRefCountedMemory::From(std::move(data)));
+ auto reply = detail::ReadReply<GetPropertyReply>(&buf);
+ ASSERT_TRUE(reply);
+ EXPECT_EQ(reply->format, 32);
+}
+
} // namespace x11
diff --git a/ui/gfx/x/gen_xproto.py b/ui/gfx/x/gen_xproto.py
index 92a4338..6043a8c 100644
--- a/ui/gfx/x/gen_xproto.py
+++ b/ui/gfx/x/gen_xproto.py
@@ -613,6 +613,13 @@
if t.is_ref_counted_memory:
if self.is_read:
+ if name == 'value' and field.parent and field.parent[1] == (
+ 'xcb', 'GetProperty'):
+ with Indent(
+ self,
+ 'if (format != 0 && format != 8 && format != 16 && format != 32) {',
+ '}'):
+ self.write('return nullptr;')
self.write('%s = buffer->ReadAndAdvance(%s);' % (name, size))
elif t.is_sized:
self.write('buf.AppendSizedBuffer(%s);' % (name))
diff --git a/ui/gfx/x/generated_protos/xproto.cc b/ui/gfx/x/generated_protos/xproto.cc
index d5e2ce0..c2d7de4 100644
--- a/ui/gfx/x/generated_protos/xproto.cc
+++ b/ui/gfx/x/generated_protos/xproto.cc
@@ -4276,6 +4276,9 @@
Pad(&buf, 12);
// value
+ if (format != 0 && format != 8 && format != 16 && format != 32) {
+ return nullptr;
+ }
value = buffer->ReadAndAdvance((value_len) * ((format) / (8)));
Align(&buf, 4);
Regression Test / PoC
diff --git a/ui/gfx/x/connection_unittest.cc b/ui/gfx/x/connection_unittest.cc
index 9116707..fc451c4f 100644
--- a/ui/gfx/x/connection_unittest.cc
+++ b/ui/gfx/x/connection_unittest.cc
@@ -123,4 +123,36 @@
connection.QueryTree(root).Sync();
}
+TEST(X11ConnectionTest, GetPropertyReplyValidation) {
+ // Simulate a malicious response with format 39.
+ // The reply length is in 4-byte units, starting from after the first 32
+ // bytes. A GetProperty reply has a fixed size of 32 bytes followed by the
+ // value.
+ std::vector<uint8_t> data(32, 0);
+ data[0] = 1; // response_type: Reply
+ data[1] = 39; // format: 39 (Invalid, should be 8, 16, or 32)
+ data[10] = 0; // length: 0
+ data[11] = 0;
+
+ ReadBuffer buf(x11::ThrowAwaySizeRefCountedMemory::From(std::move(data)));
+ auto reply = detail::ReadReply<GetPropertyReply>(&buf);
+ EXPECT_FALSE(reply);
+}
+
+TEST(X11ConnectionTest, GetPropertyReplyValid) {
+ // Simulate a valid response with format 32.
+ std::vector<uint8_t> data(32, 0);
+ data[0] = 1; // response_type: Reply
+ data[1] = 32; // format: 32
+ data[10] = 0; // length: 0
+ data[11] = 0;
+ data[16] = 0; // type: None
+ data[24] = 0; // value_len: 0
+
+ ReadBuffer buf(x11::ThrowAwaySizeRefCountedMemory::From(std::move(data)));
+ auto reply = detail::ReadReply<GetPropertyReply>(&buf);
+ ASSERT_TRUE(reply);
+ EXPECT_EQ(reply->format, 32);
+}
+
} // namespace x11
Original Bug Report
Heap OOB Read/Write in X11 GetArrayProperty via Arithmetic Mismatch
Project Fortify, an experimental security project, has identified the following potential security issue.
Overview: An arithmetic mismatch in X11 property calculations between the generated parser and the GetArrayProperty consumer can lead to a heap out-of-bounds read and write. A malicious X server can return a crafted format value, causing the consumer to calculate a larger copy size than the allocated buffer. This allows an attacker to perform a controlled OOB memory corruption in the highly-privileged browser process.
Affected files:
ui/gfx/x/connection.hui/gfx/x/generated_protos/xproto.cc
Estimated timestamp from git blame: 2025-04-21
Summary
There is a potential heap out-of-bounds (OOB) write and read in Chrome’s X11 property handling logic within ui/gfx/x. The issue stems from a discrepancy in how the byte length of property data is calculated between the auto-generated protocol parser and the GetArrayProperty template.
Note: This report was generated by our setup through static code analysis. The steps below are theoretical and a working Proof-of-Concept has not yet been executed.
Technical Details
The vulnerability is caused by an integer arithmetic mismatch when calculating the byte length of an X11 property value based on its format (bits per item) and value_len (number of items) fields.
1. Parser Logic (ui/gfx/x/generated_protos/xproto.cc):
When reading a GetPropertyReply, the auto-generated parser advances its read buffer using the following calculation (line 4279):
value = buffer->ReadAndAdvance((value_len) * ((format) / (8)));
Due to the parentheses, integer division of format by 8 occurs first, truncating any remainder. For a non-standard format like 39, 39 / 8 evaluates to 4.
2. Consumer Logic (ui/gfx/x/connection.h):
The GetArrayProperty template processes the reply and calculates the copy length (line 379):
size_t byte_len = response->value_len * response->format / 8u;
Here, multiplication occurs before division. For value_len=1000 and format=39, this evaluates as (1000 * 39) / 8 = 4875.
3. The OOB Copy (ui/gfx/x/connection.h):
The consumer allocates a destination std::vector<T> based on value_len and performs an unsafe memcpy (lines 380-382):
value->resize(response->value_len);
if (byte_len > 0u) {
UNSAFE_TODO(memcpy(value->data(), response->value->bytes(), byte_len));
}
If T=uint32_t, the vector allocates 1000 * 4 = 4000 bytes. The memcpy then attempts to copy 4875 bytes. This reads 875 bytes past the end of the X11 reply buffer (OOB read) and writes 875 bytes past the end of the vector’s backing store (OOB write).
Potential Attacker Steps
To trigger this vulnerability, an attacker would need to control the X11 server the victim connects to (e.g., via malicious local X server, X11 forwarding over SSH, or a compromised proxy).
- The attacker sets up a malicious X11 server.
- The victim launches Chrome, which connects to the X display.
- Chrome requests a standard property, such as
WM_NORMAL_HINTS, which invokesGetArrayProperty<uint32_t>. - The malicious server responds to the
GetPropertyRequestwith:format = 39value_len = 1000length = 1000(to satisfy the parser’s expected 4000-byte payload length)
- The parser successfully processes the 4000-byte payload.
GetArrayPropertyevaluates the guardresponse->format / 8u != sizeof(T). Since39 / 8 == 4 == sizeof(uint32_t), the check passes.byte_lenis calculated as 4875. Thememcpyreads 4875 bytes from the 4000-byte source and writes 4875 bytes to the 4000-byte vector, causing a heap OOB read and write.
Suggested Fix
- Strict Format Validation: The most robust fix is to strictly validate that the
formatfield provided by the X server is a standard value (i.e., exactly8,16, or32). If a non-standard format is encountered, the reply should be rejected early. - Consistent Arithmetic: Update
GetArrayPropertyto calculate the copy size exactly as the parser does, or derive it directly from the allocatedsizeof(T):size_t byte_len = response->value_len * sizeof(T);
Evaluated with Chrome root at commit: 0eb4855bda702feaaa8b899336664f97e3df88b8
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. Please feel free to reach out to me if you have concerns or feedback.