search

merbanan / rtl_433

Program to decode radio transmissions from devices on the ISM bands (and other frequencies)
GNU General Public License v2.0
5.98k stars 1.3k forks source link

lacrosse ws7000 extra checking #1189

Closed Mindavi closed 1 year ago

Mindavi commented 4 years ago

Not sure how to fix this, maybe unsigned may be used to ensure it's not undefined behaviour but defined behaviour. At least reporting it so this can be picked up eventually.

rtl_433 -y {124}07875957Cd90cc3a8a03599f3fdaef9 will trigger this, make sure ASAN is compiled in.

rtl_433 version 19.08-69-g9e04f93 branch master at 201911011341 inputs file rtl_tcp RTL-SDR
Use -h for usage help and see https://triq.org/ for documentation.
Trying conf file at "rtl_433.conf"...
Trying conf file at "/home/rick/.config/rtl_433/rtl_433.conf"...
Trying conf file at "/usr/local/etc/rtl_433/rtl_433.conf"...
Trying conf file at "/etc/rtl_433/rtl_433.conf"...

    Consider using "-M newmodel" to transition to new model keys. This will become the default someday.
    A table of changes and discussion is at https://github.com/merbanan/rtl_433/pull/986.

Registered 111 out of 141 device decoding protocols [ 1-4 8 11-12 15-17 19-21 23 25-26 29-36 38-60 63 67-71 73-100 102-103 108-116 119 121 124-128 131-141 ]
/home/rick/hobby/rtl-sdr/433mhz/rtl_433/src/devices/lacrosse_ws7000.c:200:24: runtime error: signed integer overflow: 1542000000 * 10 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/rick/hobby/rtl-sdr/433mhz/rtl_433/src/devices/lacrosse_ws7000.c:200:24 in 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
time      : 2019-11-01 21:00:13
model     : LaCrosse-WS2500-19                     id        : 81
channel   : 1            Brightness: 2084139008    Exposition: 440           MIC       : CHECKSUM

Maybe a sanity check for the brightness value might also be good, 2084139008 doesn't seem right either way.

zuckschwerdt commented 4 years ago

I guess an exponent greater that 3 won't make sense there: three digits, three zeros gives the maximum sensible range for lux. But there's also lots of unchecked BCD in there, maybe we should test all that for plausibility too?

Mindavi commented 4 years ago

Haven't lookes into this yet, but the undefined behavior is solved in ff3482e. This is now at least defined behavior, albeit probably still incorrect.

merbanan commented 1 year ago

We can not check every value in every decoder. Code should not crash or be undefined but trusting values are up to the user. We make a best effort and then leave at that but if someone wants to add more check I will not reject patches.