From fb19e42e9bcf7a8a675e8691ee6f2de9792b8718 Mon Sep 17 00:00:00 2001
From: Andrew Paseltiner <[email protected]>
Date: Tue, 14 Apr 2026 17:07:26 -0700
Subject: [PATCH] Fix heap out-of-bounds read in DeviceDataManagerX11

Refactor XInput mask utility functions to use base::span for safer
memory access and explicit bounds checking. Previously, these functions
used raw pointers without size information, leading to potential
out-of-bounds reads when processing truncated XInput event masks sent by
the X server.

Specific changes:

- Update SetXinputMask and IsXinputMaskSet in xinput_util.h to
  accept base::span<uint8_t> and base::span<const uint8_t>.
- Use base::as_byte_span, base::as_writable_byte_span, or
  base::byte_span_from_ref at all call sites (including
  DeviceDataManagerX11, TouchFactory, and various test utilities) to
  pass masks safely.
- Add unit tests in xinput_util_unittest.cc verifying safe bounds
  handling for both IsXinputMaskSet and SetXinputMask, using
  EXPECT_DEATH_IF_SUPPORTED for the latter.
- Add regression test in device_data_manager_x11_unittest.cc for
  GetEventData with truncated masks.

Fixed: 501862016
Change-Id: Ia68255583bdaf944e786dea2a64cde937e761e4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7759573
Reviewed-by: Thomas Anderson <[email protected]>
Reviewed-by: Joe Downing <[email protected]>
Commit-Queue: Andrew Paseltiner <[email protected]>
Reviewed-by: Jonathan Ross <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1614784}
---

diff --git a/remoting/host/input_monitor/local_input_monitor_x11_common.cc b/remoting/host/input_monitor/local_input_monitor_x11_common.cc
index 8ae0ad0..cc40c32 100644
--- a/remoting/host/input_monitor/local_input_monitor_x11_common.cc
+++ b/remoting/host/input_monitor/local_input_monitor_x11_common.cc
@@ -4,13 +4,18 @@
 
 #include "remoting/host/input_monitor/local_input_monitor_x11_common.h"
 
+#include "base/containers/span.h"
+
 namespace remoting {
 
 x11::Input::XIEventMask CommonXIEventMaskForRootWindow() {
   x11::Input::XIEventMask mask{};
-  ui::SetXinputMask(&mask, x11::Input::RawDeviceEvent::RawKeyPress);
-  ui::SetXinputMask(&mask, x11::Input::RawDeviceEvent::RawKeyRelease);
-  ui::SetXinputMask(&mask, x11::Input::RawDeviceEvent::RawMotion);
+  ui::SetXinputMask(base::byte_span_from_ref(mask),
+                    x11::Input::RawDeviceEvent::RawKeyPress);
+  ui::SetXinputMask(base::byte_span_from_ref(mask),
+                    x11::Input::RawDeviceEvent::RawKeyRelease);
+  ui::SetXinputMask(base::byte_span_from_ref(mask),
+                    x11::Input::RawDeviceEvent::RawMotion);
   return mask;
 }
 
diff --git a/ui/base/x/x11_user_input_monitor.cc b/ui/base/x/x11_user_input_monitor.cc
index 9cd9b69..0d02cbe7 100644
--- a/ui/base/x/x11_user_input_monitor.cc
+++ b/ui/base/x/x11_user_input_monitor.cc
@@ -4,6 +4,7 @@
 
 #include "ui/base/x/x11_user_input_monitor.h"
 
+#include "base/containers/span.h"
 #include "base/logging.h"
 #include "base/task/single_thread_task_runner.h"
 #include "ui/events/devices/x11/xinput_util.h"
@@ -78,8 +79,10 @@
   }
 
   x11::Input::XIEventMask mask{};
-  SetXinputMask(&mask, x11::Input::RawDeviceEvent::RawKeyPress);
-  SetXinputMask(&mask, x11::Input::RawDeviceEvent::RawKeyRelease);
+  SetXinputMask(base::byte_span_from_ref(mask),
+                x11::Input::RawDeviceEvent::RawKeyPress);
+  SetXinputMask(base::byte_span_from_ref(mask),
+                x11::Input::RawDeviceEvent::RawKeyRelease);
   connection_->xinput().XISelectEvents(
       {connection_->default_root(),
        {{x11::Input::DeviceId::AllMaster, {mask}}}});
diff --git a/ui/events/devices/x11/device_data_manager_x11.cc b/ui/events/devices/x11/device_data_manager_x11.cc
index 0447e08d6..0fd92a2 100644
--- a/ui/events/devices/x11/device_data_manager_x11.cc
+++ b/ui/events/devices/x11/device_data_manager_x11.cc
@@ -13,6 +13,7 @@
 #include "base/at_exit.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/containers/span.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/logging.h"
@@ -288,7 +289,7 @@
   data->clear();
   auto valuators_iter = xiev->axisvalues.begin();
   for (int i = 0; i <= valuator_count_[sourceid]; ++i) {
-    if (IsXinputMaskSet(xiev->valuator_mask.data(), i)) {
+    if (IsXinputMaskSet(base::as_byte_span(xiev->valuator_mask), i)) {
       int type = data_type_lookup_[sourceid][i];
       if (type != DT_LAST_ENTRY) {
         double valuator = Fp3232ToDouble(*valuators_iter);
@@ -337,11 +338,13 @@
   int val_index = valuator_lookup_[sourceid][type].number;
   int slot = 0;
   if (val_index >= 0) {
-    if (IsXinputMaskSet(xiev->valuator_mask.data(), val_index)) {
+    if (IsXinputMaskSet(base::as_byte_span(xiev->valuator_mask), val_index)) {
       auto valuators_iter = xiev->axisvalues.begin();
       while (val_index--) {
-        if (IsXinputMaskSet(xiev->valuator_mask.data(), val_index))
+        if (IsXinputMaskSet(base::as_byte_span(xiev->valuator_mask),
+                            val_index)) {
           ++valuators_iter;
+        }
       }
       *value = Fp3232ToDouble(*valuators_iter);
       if (IsTouchDataType(type)) {
@@ -409,11 +412,13 @@
   int horizontal_id = scroll_data_.at(sourceid).horizontal.number;
   int vertical_id = scroll_data_.at(sourceid).vertical.number;
   return (horizontal_id != -1 &&
-                  IsXinputMaskSet(xievent->valuator_mask.data(), horizontal_id)
+                  IsXinputMaskSet(base::as_byte_span(xievent->valuator_mask),
+                                  horizontal_id)
               ? SCROLL_TYPE_HORIZONTAL
               : 0) |
          (vertical_id != -1 &&
-                  IsXinputMaskSet(xievent->valuator_mask.data(), vertical_id)
+                  IsXinputMaskSet(base::as_byte_span(xievent->valuator_mask),
+                                  vertical_id)
               ? SCROLL_TYPE_VERTICAL
               : 0);
 }
@@ -450,7 +455,8 @@
     return false;
   }
   const int idx = valuator_lookup_.at(sourceid)[type].number;
-  return (idx >= 0) && IsXinputMaskSet(xiev->valuator_mask.data(), idx);
+  return (idx >= 0) &&
+         IsXinputMaskSet(base::as_byte_span(xiev->valuator_mask), idx);
 }
 
 bool DeviceDataManagerX11::IsScrollEvent(const x11::Event& x11_event) const {
@@ -539,8 +545,9 @@
   const int vertical_number = info->vertical.number;
 
   for (int i = 0; i <= valuator_count_[sourceid]; ++i) {
-    if (!IsXinputMaskSet(xiev->valuator_mask.data(), i))
+    if (!IsXinputMaskSet(base::as_byte_span(xiev->valuator_mask), i)) {
       continue;
+    }
     auto valuator = Fp3232ToDouble(*valuators_iter);
     if (i == horizontal_number)
       *x_offset = ExtractAndUpdateScrollOffset(&info->horizontal, valuator);
@@ -704,14 +711,15 @@
     double value) {
   auto device = devev->deviceid;
   int index = valuator_lookup_[device][type].number;
-  CHECK(!IsXinputMaskSet(devev->valuator_mask.data(), index));
+  CHECK(!IsXinputMaskSet(base::as_byte_span(devev->valuator_mask), index));
   CHECK(index >= 0 && index < valuator_count_[device]);
-  SetXinputMask(devev->valuator_mask.data(), index);
+  SetXinputMask(base::as_writable_byte_span(devev->valuator_mask), index);
 
   x11::Input::Fp3232* valuators = devev->axisvalues.data();
   for (int i = 0; i < index; ++i) {
-    if (IsXinputMaskSet(devev->valuator_mask.data(), i))
+    if (IsXinputMaskSet(base::as_byte_span(devev->valuator_mask), i)) {
       UNSAFE_TODO(valuators++);
+    }
   }
   for (int i = DT_LAST_ENTRY - 1; i > valuators - devev->axisvalues.data();
        --i) {
diff --git a/ui/events/devices/x11/touch_factory_x11.cc b/ui/events/devices/x11/touch_factory_x11.cc
index b99a2b1..847d70d9 100644
--- a/ui/events/devices/x11/touch_factory_x11.cc
+++ b/ui/events/devices/x11/touch_factory_x11.cc
@@ -11,6 +11,7 @@
 
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/containers/span.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/strings/string_number_conversions.h"
@@ -206,27 +207,27 @@
 
   x11::Input::EventMask mask{x11::Input::DeviceId::AllMaster};
   mask.mask.push_back({});
-  auto* mask_data = mask.mask.data();
+  auto mask_span = base::as_writable_byte_span(mask.mask);
 
-  SetXinputMask(mask_data, x11::Input::CrossingEvent::Enter);
-  SetXinputMask(mask_data, x11::Input::CrossingEvent::Leave);

diff --git a/ui/events/test/events_test_utils_x11.cc b/ui/events/test/events_test_utils_x11.cc
index ab6ba12c..cac3297 100644
--- a/ui/events/test/events_test_utils_x11.cc
+++ b/ui/events/test/events_test_utils_x11.cc
@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/check_op.h"
+#include "base/containers/span.h"
 #include "base/notreached.h"
 #include "ui/events/devices/x11/touch_factory_x11.h"
 #include "ui/events/devices/x11/xinput_util.h"
@@ -207,7 +208,8 @@
   dev_event->detail = XButtonEventButton(type, flags);
   dev_event->event_x = ToFp1616(location.x()),
   dev_event->event_y = ToFp1616(location.y()),
-  SetXinputMask(dev_event->button_mask.data(), XButtonEventButton(type, flags));
+  SetXinputMask(base::as_writable_byte_span(dev_event->button_mask),
+                XButtonEventButton(type, flags));
 
   // Setup an empty valuator list for generic button events.
   SetUpValuators(std::vector<Valuator>());
diff --git a/ui/ozone/platform/x11/test/device_data_manager_x11_unittest.cc b/ui/ozone/platform/x11/test/device_data_manager_x11_unittest.cc
index 15bbd53..648a69c 100644
--- a/ui/ozone/platform/x11/test/device_data_manager_x11_unittest.cc
+++ b/ui/ozone/platform/x11/test/device_data_manager_x11_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <vector>
 
+#include "base/containers/span.h"
 #include "base/memory/raw_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/devices/device_hotplug_event_observer.h"
@@ -176,5 +177,35 @@
   EXPECT_EQ(2u, devices.size());
 }
 
+// Tests that GetEventData handles cases where the event's valuator_mask is
+// shorter than the registered valuator's index.
+// Regression test for crbug.com/501862016.
+TEST_F(DeviceDataManagerX11Test, GetEventDataShortMask) {
+  DeviceDataManagerX11* manager = DeviceDataManagerX11::GetInstance();
+  const auto device_id = static_cast<x11::Input::DeviceId>(1);
+
+  // Initialize a device with some valuators. SetDeviceListForTest will
+  // initialize them with indices starting from 0.
+  // Touch major/minor/orientation/pressure/x/y/tracking_id/raw_timestamp
+  // are 8 valuators (indices 0-7).
+  manager->SetDeviceListForTest({static_cast<int>(device_id)}, {}, {});
+
+  // Manually create an event with an empty mask.
+  x11::Event event(false, x11::Input::DeviceEvent{
+                              .opcode = x11::Input::DeviceEvent::Motion,
+                              .deviceid = device_id,
+                              .sourceid = device_id,
+                              .valuator_mask = {},
+                              .axisvalues = {},
+                          });
+
+  double value = -1.0;
+  // This should not crash and should return false because any index is OOB
+  // for an empty mask.
+  EXPECT_FALSE(manager->GetEventData(
+      event, DeviceDataManagerX11::DT_TOUCH_MAJOR, &value));
+  EXPECT_EQ(value, -1.0);
+}
+
 }  // namespace test
 }  // namespace ui
diff --git a/ui/ozone/platform/x11/test/xinput_util_unittest.cc b/ui/ozone/platform/x11/test/xinput_util_unittest.cc
new file mode 100644
index 0000000..07d56f84
--- /dev/null
+++ b/ui/ozone/platform/x11/test/xinput_util_unittest.cc
@@ -0,0 +1,70 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/events/devices/x11/xinput_util.h"
+
+#include <vector>
+
+#include "base/containers/span.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/x/xinput.h"
+
+namespace ui {
+
+// Regression test for crbug.com/501862016.
+TEST(XInputUtilTest, IsXinputMaskSetBoundsCheck) {
+  std::vector<uint32_t> mask = {1};  // 4 bytes, covering opcodes 0-31
+  auto mask_bytes = base::as_byte_span(mask);
+
+  // Opcode 0 is set (first bit of 1)
+  EXPECT_TRUE(IsXinputMaskSet(mask_bytes, 0));
+
+  // Opcode 1 is not set
+  EXPECT_FALSE(IsXinputMaskSet(mask_bytes, 1));
+
+  // Opcode 31 is not set
+  EXPECT_FALSE(IsXinputMaskSet(mask_bytes, 31));
+
+  // Opcode 32 is out of bounds for a 4-byte mask
+  EXPECT_FALSE(IsXinputMaskSet(mask_bytes, 32));
+
+  // Opcode 1000 is way out of bounds
+  EXPECT_FALSE(IsXinputMaskSet(mask_bytes, 1000));
+}
+
+// Regression test for crbug.com/501862016.
+TEST(XInputUtilTest, SetXinputMask) {
+  std::vector<uint32_t> mask = {0};  // 4 bytes, covering opcodes 0-31
+
+  // Setting opcode 0 should work
+  SetXinputMask(base::as_writable_byte_span(mask), 0);
+  EXPECT_EQ(mask[0], 1u);
+  EXPECT_TRUE(IsXinputMaskSet(base::as_byte_span(mask), 0));
+}
+
+// Regression test for crbug.com/501862016.
+TEST(XInputUtilTest, SetXinputMaskDeathTest) {
+  std::vector<uint32_t> mask = {0};  // 4 bytes, covering opcodes 0-31
+
+  // Setting opcode 32 (out of bounds) should CHECK/crash
+  EXPECT_DEATH_IF_SUPPORTED(
+      SetXinputMask(base::as_writable_byte_span(mask), 32), "");
+}
+
+// Regression test for crbug.com/501862016.
+TEST(XInputUtilTest, XIEventMaskSpan) {
+  x11::Input::XIEventMask xi_mask{};
+
+  // Initially nothing set
+  EXPECT_FALSE(IsXinputMaskSet(base::byte_span_from_ref(xi_mask), 1));
+
+  // Set some bit
+  SetXinputMask(base::byte_span_from_ref(xi_mask), 1);
+  EXPECT_TRUE(IsXinputMaskSet(base::byte_span_from_ref(xi_mask), 1));
+
+  // Out of bounds for int-sized mask (usually 32 bits)
+  EXPECT_FALSE(IsXinputMaskSet(base::byte_span_from_ref(xi_mask), 100));
+}
+
+}  // namespace ui