Merge pull request #326 from MoonModules/dxm_hotfix

hotfix for DMX-serial output (prevents random crashes caused by array bounds violations)
* Align ESPDMX and SparkFunDMX with upstream code
* Prevent driver compilation when WLED_ENABLE_DMX is not set (saves ~1KB RAM)
* Protect against writing outside of array bounds
* Align start byte handling between ESPDMX and SparkFunDMX driver.

Author: softhack007

wled00/dmx_input.cpp

@@ -1,7 +1,7 @@
 #include "wled.h"
 
 #ifdef WLED_ENABLE_DMX_INPUT
-#pragma message "DMX physical input driver enabled"
+#pragma message "DMX physical input driver (esp-dmx) enabled"
 
 #ifdef ESP8266
 #error DMX input is only supported on ESP32

wled00/dmx_output.cpp

@@ -11,7 +11,7 @@
  */
 
 #ifdef WLED_ENABLE_DMX
-#pragma message "DMX network output enabled"
+#pragma message "DMX output enabled"
 
 #define MAX_DMX_RATE 44 // max DMX update rate in Hz
 
@@ -99,7 +99,7 @@ void handleDMXOutput()
 }
 
 void initDMXOutput() {
- #ifdef ESP8266
+ #if defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S2)
   dmx.init(512);        // initialize with bus length
  #else
   dmx.initWrite(512);  // initialize with bus length

wled00/src/dependencies/dmx/ESPDMX.cpp

@@ -11,12 +11,13 @@
 // - - - - -
 
 /* ----- LIBRARIES ----- */
-#ifdef ESP8266
+#if defined(WLED_ENABLE_DMX) && (defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S2))
 
 #include <Arduino.h>
 
 #include "ESPDMX.h"
 
+#pragma message "using ESPDMX"
 
 
 #define dmxMaxChannel  512
@@ -30,8 +31,8 @@
 bool dmxStarted = false;
 int sendPin = 2;		//default on ESP8266
 
-//DMX value array and size. Entry 0 will hold startbyte
-uint8_t dmxDataStore[dmxMaxChannel] = {};
+//DMX value array and size. Entry 0 will hold startbyte, so we need 512+1 elements
+uint8_t dmxDataStore[dmxMaxChannel+1] = {};
 int channelSize;
 
 
@@ -72,6 +73,7 @@ void DMXESPSerial::write(int Channel, uint8_t value) {
 
   if (Channel < 1) Channel = 1;
   if (Channel > channelSize) Channel = channelSize;
+  if (Channel > dmxMaxChannel) Channel = dmxMaxChannel; // WLEDMM protect against array bounds violation
   if (value < 0) value = 0;
   if (value > 255) value = 255;
 
@@ -98,12 +100,12 @@ void DMXESPSerial::update() {
   //send data
   Serial1.begin(DMXSPEED, DMXFORMAT);
   digitalWrite(sendPin, LOW);
-  Serial1.write(dmxDataStore, channelSize);
+  Serial1.write(dmxDataStore, min(dmxMaxChannel+1, channelSize+1));
   Serial1.flush();
   delay(1);
   Serial1.end();
 }
 
 // Function to update the DMX bus
 
-#endif
+#endif

wled00/src/dependencies/dmx/SparkFunDMX.cpp

@@ -14,14 +14,16 @@ Distributed as-is; no warranty is given.
 ******************************************************************************/
 
 /* ----- LIBRARIES ----- */
-#if defined(ARDUINO_ARCH_ESP32)
+#if defined(ARDUINO_ARCH_ESP32) && defined(WLED_ENABLE_DMX)
 
 #include <Arduino.h>
 #if !defined(CONFIG_IDF_TARGET_ESP32C3)  && !defined(CONFIG_IDF_TARGET_ESP32S2)
 
 #include "SparkFunDMX.h"
 #include <HardwareSerial.h>
 
+#pragma message "using SparkFunDMX"
+
 #define dmxMaxChannel  512
 #define defaultMax 32
 
@@ -34,8 +36,8 @@ static const int enablePin = -1;		// disable the enable pin because it is not ne
 static const int rxPin = -1;       // disable the receiving pin because it is not needed - softhack007: Pin=-1 means "use default" not "disable"
 static const int txPin = 2;        // transmit DMX data over this pin (default is pin 2)
 
-//DMX value array and size. Entry 0 will hold startbyte
-static uint8_t dmxData[dmxMaxChannel] = { 0 };
+//DMX value array and size. Entry 0 will hold startbyte, so we need 512+1 elements
+static uint8_t dmxData[dmxMaxChannel+1] = { 0 };
 static int chanSize = 0;
 #if !defined(DMX_SEND_ONLY)
 static int currentChannel = 0;
@@ -121,15 +123,17 @@ void SparkFunDMX::initWrite (int chanQuant) {
 #if !defined(DMX_SEND_ONLY)
 // Function to read DMX data
 uint8_t SparkFunDMX::read(int Channel) {
+  if ((Channel > dmxMaxChannel) || (Channel < 1)) return 0; // WLEDMM prevent array out-of-bounds access
   if (Channel > chanSize) Channel = chanSize;
   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;
+  if (Channel < 1) Channel = 1;
+  if (Channel+1 > chanSize) chanSize = min(dmxMaxChannel +1, Channel+1); // WLEDMM "+1" as we need to account for start byte
+  if (Channel > dmxMaxChannel) Channel = dmxMaxChannel;                  // WLEDMM prevent array out-of-bounds access
   dmxData[0] = 0;
   dmxData[Channel] = value; //add one to account for start byte
 }
@@ -149,7 +153,7 @@ void SparkFunDMX::update() {
 
     //Send DMX data
     DMXSerial.begin(DMXSPEED, DMXFORMAT, rxPin, txPin);//Begin the Serial port
-    DMXSerial.write(dmxData, chanSize);
+    DMXSerial.write(dmxData, min(dmxMaxChannel+1, chanSize)); // send max 513 bytes = start byte + 512 channels
     DMXSerial.flush();
     DMXSerial.end();//clear our DMX array, end the Hardware Serial port
   }
@@ -160,9 +164,11 @@ void SparkFunDMX::update() {
 	{
 		while (DMXSerial.available())
 		{
-			dmxData[currentChannel++] = DMXSerial.read();
+		  uint8_t newdata = DMXSerial.read();
+		  if (currentChannel <= dmxMaxChannel)
+		    dmxData[currentChannel++] = newdata;
 		}
-	if (currentChannel > chanSize) //Set the channel counter back to 0 if we reach the known end size of our packet
+	if ((currentChannel > chanSize) || (currentChannel > dmxMaxChannel)) //Set the channel counter back to 0 if we reach the known end size of our packet
 	{
 
       portENTER_CRITICAL(&timerMux);

wled00/wled.h

@@ -164,9 +164,9 @@
 #endif
 
 #ifdef WLED_ENABLE_DMX
- #ifdef ESP8266
+  #if defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S2)
   #include "src/dependencies/dmx/ESPDMX.h"
- #else //ESP32
+ #else //ESP32 or ESP32-S3
   #include "src/dependencies/dmx/SparkFunDMX.h"
  #endif
 #endif
@@ -468,9 +468,9 @@ WLED_GLOBAL bool arlsDisableGammaCorrection _INIT(true);          // activate if
 WLED_GLOBAL bool arlsForceMaxBri _INIT(false);                    // enable to force max brightness if source has very dark colors that would be black
 
 #ifdef WLED_ENABLE_DMX
- #ifdef ESP8266
+ #if defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S2)
   WLED_GLOBAL DMXESPSerial dmx;
- #else //ESP32
+ #else //ESP32 or ESP32-S3
   WLED_GLOBAL SparkFunDMX dmx;
  #endif
 WLED_GLOBAL uint16_t e131ProxyUniverse _INIT(0);                  // output this E1.31 (sACN) / ArtNet universe via MAX485 (0 = disabled)