openshwprojects / OpenBK7231T_App

Open source firmware (Tasmota/Esphome replacement) for BK7231T, BK7231N, BL2028N, T34, XR809, W800/W801, W600/W601 and BL602
https://openbekeniot.github.io/webapp/devicesList.html
1.33k stars 227 forks source link

IRSend args buffer is too small for inputs #1051

Open Error-Gap opened 5 months ago

Error-Gap commented 5 months ago

Describe the bug "IR_Send_Cmd" in file driver/drv_ir.cpp uses a temporary buffer "args" for arguments that is only 20 chars (19 + a zero in the last character) This copies data in from commands but is insufficient for larger IRSend commands including those for protocols with larger names, in particular Kaseikyo which supports various vendor subtypes under protocol names names

These commands could exceed 30 characters for the arguments i.e. IRSend Kaseikyo_Mitsubishi 0x330 0x2d 1

The buffer size for the actual command textbox actually seems to be 128 characters so 20 is drastically truncating the potential input, though the likely input is less due to the command-format and protocol-names. If RAW IRSend capabilities are added, the raw strings could get even larger.

I believe that increasing the size of the "args" character array can potentially fix this, though it's worth validating that this doesn't result in a buffer overflow further down.

Firmware:

To Reproduce Steps to reproduce the behavior:

  1. Open a second window with the App open to the "logs" section. Select only the "IR" log feature
  2. Go back to the main interface window, leaving the second window open to see logs
  3. Under the "config" section (WebUI) go to "Execute Command"
  4. Attempt IRSend with any larger protocol name, i.e. Kaseikyo_Denon or Kaseikyo_Mitsubishi, e.g. IRSend Kaseikyo_Denon 414 12 IRSend Kaseikyo_Mitsubishi 414 12
  5. Flip back to the logs window
  6. Note that the output string is truncated, which snips the addr and protocol bits as well Info:IR:IR send Kaseikyo_Denon 414 protocol 12 addr 0x414 cmd 0x0 repeats 0 bits override 0 Error:IR:IRSend cmnd not valid [Kaseikyo_Mitsubishi] not like [NEC-0-1A] or [NEC 0 1A 1].
  7. Data will be lots in the address and cmd fields, resulting in incorrect values or an error depending on where it is cut off

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Ref: https://www.elektroda.com/rtvforum/viewtopic.php?p=20932660#20932660

openshwprojects commented 4 months ago

Thx, buffer is now increased now in both forks

Error-Gap commented 4 months ago

Thanks @openshwprojects

I did test this and it worked so far the input buffer parsing through as expected, but then rather into further issues where the device still refused to actually send any IR codes.

Further investigation showed that the Kaseikyo variety remotes use the "sendPulseDistanceWidthFromArray" function, which would dead-end (no code) under the SpoofIrSender object.

I've managed to modify drv_ir.h and drv_ir.c so that it passes up the a reference to the "myIRsend" object to this class, which then pushed the function back down through that to the original IRsend functions from IRSend.hpp

I've compiled and tested the changes on my device and this worked to get it successfully sending codes emulating the keiseikyo denon remote.

Please see the attached patch. Assuming all compiles properly for you after applying it then this should be a functional update to fix this issue.

Cheers,

ErrorGap

(updated) drv_ir.2024-02-19.patch.gz

p.s. If there is a better way contribute updates/patches please let me know. TBH it's been quite a long time since I've poked at anyone's public code nor contributed to projects that I wasn't part of the development team etc for.

openshwprojects commented 4 months ago

Thanks, can you just open a pull request so I can automatically merge it? It can be easily done via Github Windows client.

Error-Gap commented 4 months ago

Done. You should see the pull request now.