OpenBK7231T_App icon indicating copy to clipboard operation
OpenBK7231T_App copied to clipboard

IRSend args buffer is too small for inputs

Open Error-Gap opened this issue 1 year ago • 4 comments

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

  • Kaseikyo_Denon
  • Kaseikyo_Sharp
  • Kaseikyo_JVC
  • Kaseikyo_Mitsubishi

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:

  • Version 1.17.427
  • Device? Tuya IRC003
  • Chip/model: BK7231N openbeken_error_irsend

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

Error-Gap avatar Jan 28 '24 07:01 Error-Gap

Thx, buffer is now increased now in both forks

openshwprojects avatar Feb 02 '24 10:02 openshwprojects

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.

Error-Gap avatar Feb 20 '24 05:02 Error-Gap

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

openshwprojects avatar Feb 22 '24 07:02 openshwprojects

Done. You should see the pull request now.

Error-Gap avatar Feb 23 '24 06:02 Error-Gap