From bf61b3e22b2bb25dc9aed76103488eb7ebc47bb5 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Thu, 30 Nov 2023 08:02:08 -0800 Subject: [PATCH] wcmUSB: Correct bounds check of maximum button number Automated test runs have detected the following issue while running UBSan checks: ~~~ ../src/wcmUSB.c:1372:11: runtime error: left shift of 1 by 31 places cannot be represented in type 'int' #0 0x7f0444bcbd8c in mod_buttons ../src/wcmUSB.c:1372 #1 0x7f0444bd7f26 in test_mod_buttons ../src/wcmUSB.c:2090 #2 0x7f0444bfcea7 in wcm_run_tests ../test/wacom-test-suite.c:46 #3 0x56204d77b405 in main ../test/wacom-tests.c:44 #4 0x7f0448625082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) #5 0x56204d77b1cd in _start (/home/runner/work/xf86-input-wacom/xf86-input-wacom/builddir/wacom-tests+0x11cd) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/wcmUSB.c:1372:11 in ~~~ While the faulty line has some protection against an excessively-large value of 'btn', the bounds are incorrect. A button number of 32 would be allowed by the existing check but would also lead to undefined behavior. This commit modifies the bounds to properly fit the condition. Link: https://github.com/linuxwacom/xf86-input-wacom/actions/runs/7049012015/job/19186502078 Signed-off-by: Jason Gerecke --- src/wcmUSB.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wcmUSB.c b/src/wcmUSB.c index b7b3d060..40649776 100644 --- a/src/wcmUSB.c +++ b/src/wcmUSB.c @@ -1361,7 +1361,7 @@ mod_buttons(WacomCommonPtr common, unsigned int buttons, unsigned int btn, Bool { unsigned int mask; - if (btn >= sizeof(int) * 8) + if (btn >= sizeof(int) * 8 - 1) { wcmLogCommonSafe(common, W_ERROR, "%s: Invalid button number %u. Insufficient storage\n", @@ -2085,7 +2085,7 @@ static int usbProbeKeys(WacomDevicePtr priv) TEST_CASE(test_mod_buttons) { WacomCommonRec common = {0}; - for (size_t i = 0; i < sizeof(int) * 8; i++) + for (size_t i = 0; i < sizeof(int) * 8 - 1; i++) { unsigned int buttons = mod_buttons(&common, 0, i, 1); assert(buttons == (1u << i)); @@ -2093,6 +2093,7 @@ TEST_CASE(test_mod_buttons) assert(buttons == 0); } + assert(mod_buttons(&common, 0, sizeof(int) * 8 - 1, 1) == 0); assert(mod_buttons(&common, 0, sizeof(int) * 8, 1) == 0); }