search

linuxwacom / xf86-input-wacom

X.Org driver for Wacom devices
356 stars 45 forks source link

wcmUSB: Correct bounds check of maximum button number #329

Closed jigpu closed 8 months ago

jigpu commented 8 months ago

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

whot commented 8 months ago

merged, thanks!

whot commented 8 months ago

Ok, I'm still staring at this and... this can't be right, button 31 is a legitimate button so the issue is elsewhere - and I found it, see #330. Sorry about that, my brain didn't really switch on until after the merge.