Avoid calling memcpy with invalid destination

Deep copy of a HardwareState with zero fingers causes memcpy call
to copy zero bytes to a null destination. This behaviour is undefined
 and UndefinedBehaviorSanitizer on Android complains about it. This
change adds a check to avoid this unnecessary call.

BUG=b:302505955
TEST=atest libchrome-gestures_test

Change-Id: I037214e42ac9299260f4c4ed7eb09adcd5a7c06c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/gestures/+/5054360
Commit-Queue: Harry Cutts <hcutts@chromium.org>
Reviewed-by: Harry Cutts <hcutts@chromium.org>
Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com>
Tested-by: Arpit Singh <arpitks@google.com>

2 files changed, 49 insertions, 1 deletions

diff --git a/src/gestures.cc b/src/gestures.cc
index 424a843..b4d5271 100644
--- a/src/gestures.cc
+++ b/src/gestures.cc
@@ -203,7 +203,11 @@ void HardwareState::DeepCopy(const HardwareState& that,
   buttons_down = that.buttons_down;
   touch_cnt = that.touch_cnt;
   finger_cnt = min(that.finger_cnt, max_finger_cnt);
-  memcpy(fingers, that.fingers, finger_cnt * sizeof(FingerState));
+  if(that.fingers != nullptr) {
+    memcpy(fingers, that.fingers, finger_cnt * sizeof(FingerState));
+  } else if (finger_cnt > 0) {
+    Err("HardwareState with no finger data but %d finger count", finger_cnt);
+  }
   rel_x = that.rel_x;
   rel_y = that.rel_y;
   rel_wheel = that.rel_wheel;
diff --git a/src/gestures_unittest.cc b/src/gestures_unittest.cc
index ed4d61c..6b00902 100644
--- a/src/gestures_unittest.cc
+++ b/src/gestures_unittest.cc
@@ -527,4 +527,48 @@ TEST(GesturesTest, HardwareStateToStringTest) {
   return;
 }
 
+TEST(GesturesTest, HardwareStateDeepCopyWithFingersTest) {
+  FingerState fingerStates[] = {
+    { 1.0, 2.0, 3.0, 4.5, 30.0, 11.0, 20.0, 30.0, 14, 0 },
+    { 1.5, 2.5, 3.5, 5.0, 30.5, 11.5, 20.5, 30.5, 15, 0 }
+  };
+  const HardwareState hardwareState = make_hwstate(1.123, 1, 2, 2, fingerStates);
+
+  HardwareState hardwareStateCopy;
+  hardwareStateCopy.fingers = new FingerState[hardwareState.finger_cnt];
+  hardwareStateCopy.DeepCopy(hardwareState, hardwareState.finger_cnt);
+
+  EXPECT_EQ(hardwareStateCopy.String(), hardwareState.String());
+  delete[] hardwareStateCopy.fingers;
+}
+
+TEST(GesturesTest, HardwareStateDeepCopyWithoutFingersTest) {
+  const HardwareState hardwareState = make_hwstate(1.123, 1, 0, 2, nullptr);
+
+  HardwareState hardwareStateCopy;
+  hardwareStateCopy.DeepCopy(hardwareState, hardwareState.finger_cnt);
+
+  EXPECT_EQ(hardwareStateCopy.String(), hardwareState.String());
+}
+
+TEST(GesturesTest, InvalidHardwareStateDeepCopyTest) {
+  // 2 finger_cnt without any fingersState(s) specified
+  const HardwareState invalidHardwareState = make_hwstate(1.123, 1, 2, 2, nullptr);
+
+  HardwareState hardwareStateCopy;
+  hardwareStateCopy.DeepCopy(invalidHardwareState, invalidHardwareState.finger_cnt);
+
+  EXPECT_EQ(invalidHardwareState.timestamp, hardwareStateCopy.timestamp);
+  EXPECT_EQ(invalidHardwareState.buttons_down, hardwareStateCopy.buttons_down);
+  EXPECT_EQ(invalidHardwareState.finger_cnt, hardwareStateCopy.finger_cnt);
+  EXPECT_EQ(invalidHardwareState.touch_cnt, hardwareStateCopy.touch_cnt);
+  EXPECT_EQ(invalidHardwareState.fingers, hardwareStateCopy.fingers);
+  EXPECT_EQ(invalidHardwareState.rel_x, hardwareStateCopy.rel_x);
+  EXPECT_EQ(invalidHardwareState.rel_y, hardwareStateCopy.rel_y);
+  EXPECT_EQ(invalidHardwareState.rel_wheel, hardwareStateCopy.rel_wheel);
+  EXPECT_EQ(invalidHardwareState.rel_wheel_hi_res, hardwareStateCopy.rel_wheel_hi_res);
+  EXPECT_EQ(invalidHardwareState.rel_hwheel, hardwareStateCopy.rel_wheel);
+  EXPECT_EQ(invalidHardwareState.msc_timestamp, hardwareStateCopy.msc_timestamp);
+}
+
 }  // namespace gestures