Skip to content

Fixed bug of ESP32 crashing when using DMX out #5316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Nuet0815 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed bug of ESP32 crashing when using DMX out #5316

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions wled00/src/dependencies/dmx/SparkFunDMX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ Distributed as-is; no warranty is given.
#include <HardwareSerial.h>

#define dmxMaxChannel 512
#define defaultMax 32

#define DMXSPEED 250000
#define DMXFORMAT SERIAL_8N2
Expand Down Expand Up @@ -90,7 +89,7 @@ void SparkFunDMX::initRead(int chanQuant) {
_READWRITE = _READ;
if (chanQuant > dmxMaxChannel || chanQuant <= 0)
{
chanQuant = defaultMax;
chanQuant = dmxMaxChannel;
}
chanSize = chanQuant;
if (enablePin >= 0) {
Expand All @@ -106,7 +105,7 @@ void SparkFunDMX::initWrite (int chanQuant) {

_READWRITE = _WRITE;
if (chanQuant > dmxMaxChannel || chanQuant <= 0) {
chanQuant = defaultMax;
chanQuant = dmxMaxChannel;
}

chanSize = chanQuant + 1; //Add 1 for start code
Expand All @@ -121,17 +120,15 @@ void SparkFunDMX::initWrite (int chanQuant) {
#if !defined(DMX_SEND_ONLY)
// Function to read DMX data
uint8_t SparkFunDMX::read(int Channel) {
if (Channel > chanSize) Channel = chanSize;
if ((Channel < 1) || (Channel > chanSize)) return 0;
return(dmxData[Channel - 1]); //subtract one to account for start byte
}
#endif

// Function to send DMX data
void SparkFunDMX::write(int Channel, uint8_t value) {
if (Channel < 0) Channel = 0;
if (Channel > chanSize) chanSize = Channel;
dmxData[0] = 0;
dmxData[Channel] = value; //add one to account for start byte
if ((Channel < 1) || (Channel > chanSize)) return;
dmxData[Channel] = value; //start byte at dmxData[0]
}
Comment on lines 129 to 132
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Off-by-one error in bounds check allows potential buffer overflow.

In write mode, chanSize = chanQuant + 1. When chanQuant = 512 (max), chanSize = 513. The check Channel > chanSize evaluates Channel > 513, which allows Channel = 513 to pass, resulting in dmxData[513] — an out-of-bounds write (array indices are 0–512).

Proposed fix 
 void SparkFunDMX::write(int Channel, uint8_t value) {
-  if ((Channel < 1) || (Channel > chanSize)) return;
+  if ((Channel < 1) || (Channel >= chanSize)) return;
   dmxData[Channel] = value; //start byte at dmxData[0]
 }

Based on learnings, in WLED the priority is to prevent out-of-bounds memory access.

🤖 Prompt for AI Agents 
In `@wled00/src/dependencies/dmx/SparkFunDMX.cpp` around lines 129 - 132, The
write method in SparkFunDMX::write has an off-by-one bounds check allowing
Channel == chanSize which writes past the end of dmxData; update the condition
to reject Channel values >= chanSize (i.e., ensure valid Channel is
1..chanSize-1) so accesses to dmxData[Channel] cannot overflow; modify the check
that currently uses (Channel > chanSize) to use (Channel >= chanSize)
(referencing SparkFunDMX::write, Channel, chanSize, chanQuant and dmxData).



Expand Down