From e7f07f5bfce9c1131ee82567b8a22dbf84173e3b Mon Sep 17 00:00:00 2001
From: Frank <91616163+softhack007@users.noreply.github.com>
Date: Mon, 5 Dec 2022 17:04:54 +0100
Subject: [PATCH] pinmanager robustness improvement

make sure that array bounds are not violated in pinManager class.
---
 wled00/pin_manager.cpp | 10 ++++++++--
 wled00/pin_manager.h   |  6 ++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/wled00/pin_manager.cpp b/wled00/pin_manager.cpp
index fb1d3a99..f93eb54b 100644
--- a/wled00/pin_manager.cpp
+++ b/wled00/pin_manager.cpp
@@ -107,8 +107,9 @@ bool PinManagerClass::allocateMultiplePins(const managed_pin_type * mptArray, by
     }
     if (!isPinOk(gpio, mptArray[i].isOutput)) {
       #ifdef WLED_DEBUG
-      DEBUG_PRINT(F("PIN ALLOC: Invalid pin attempted to be allocated: "));
+      DEBUG_PRINT(F("PIN ALLOC: Invalid pin attempted to be allocated: GPIO "));
       DEBUG_PRINT(gpio);
+      DEBUG_PRINT(" as "); DEBUG_PRINT(mptArray[i].isOutput ? "output": "input");
       DEBUG_PRINTLN(F(""));
       #endif
       shouldFail = true;
@@ -142,6 +143,9 @@ bool PinManagerClass::allocateMultiplePins(const managed_pin_type * mptArray, by
       // as this can greatly simplify configuration arrays
       continue;
     }
+    if (gpio >= WLED_NUM_PINS) 
+      continue; // other unexpected GPIO => avoid array bounds violation
+
     byte by = gpio >> 3;
     byte bi = gpio - 8*by;
     bitWrite(pinAlloc[by], bi, true);
@@ -160,7 +164,7 @@ bool PinManagerClass::allocateMultiplePins(const managed_pin_type * mptArray, by
 bool PinManagerClass::allocatePin(byte gpio, bool output, PinOwner tag)
 {
   // HW I2C & SPI pins have to be allocated using allocateMultiplePins variant since there is always SCL/SDA pair
-  if (!isPinOk(gpio, output) || tag==PinOwner::HW_I2C || tag==PinOwner::HW_SPI) {
+  if (!isPinOk(gpio, output) || (gpio >= WLED_NUM_PINS) || tag==PinOwner::HW_I2C || tag==PinOwner::HW_SPI) {
     #ifdef WLED_DEBUG
     if (gpio < 255) {  // 255 (-1) is the "not defined GPIO"
       if (!isPinOk(gpio, output)) {
@@ -209,6 +213,7 @@ bool PinManagerClass::isPinAllocated(byte gpio, PinOwner tag)
 {
   if (!isPinOk(gpio, false)) return true;
   if ((tag != PinOwner::None) && (ownerTag[gpio] != tag)) return false;
+  if (gpio >= WLED_NUM_PINS) return false; // catch error case, to avoid array out-of-bounds access
   byte by = gpio >> 3;
   byte bi = gpio - (by<<3);
   return bitRead(pinAlloc[by], bi);
@@ -265,6 +270,7 @@ bool PinManagerClass::isPinOk(byte gpio, bool output)
 }
 
 PinOwner PinManagerClass::getPinOwner(byte gpio) {
+  if (gpio >= WLED_NUM_PINS) return PinOwner::None; // catch error case, to avoid array out-of-bounds access
   if (!isPinOk(gpio, false)) return PinOwner::None;
   return ownerTag[gpio];
 }
diff --git a/wled00/pin_manager.h b/wled00/pin_manager.h
index a2c0e36b..af473ca2 100644
--- a/wled00/pin_manager.h
+++ b/wled00/pin_manager.h
@@ -66,12 +66,14 @@ static_assert(0u == static_cast<uint8_t>(PinOwner::None), "PinOwner::None must b
 class PinManagerClass {
   private:
   #ifdef ESP8266
+  #define WLED_NUM_PINS 17
   uint8_t pinAlloc[3] = {0x00, 0x00, 0x00}; //24bit, 1 bit per pin, we use first 17bits
-  PinOwner ownerTag[17] = { PinOwner::None };
+  PinOwner ownerTag[WLED_NUM_PINS] = { PinOwner::None };
   #else
+  #define WLED_NUM_PINS 50
   uint8_t pinAlloc[7] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; // 56bit, 1 bit per pin, we use 50 bits on ESP32-S3
   uint8_t ledcAlloc[2] = {0x00, 0x00}; //16 LEDC channels
-  PinOwner ownerTag[50] = { PinOwner::None }; // new MCU's have up to 50 GPIO
+  PinOwner ownerTag[WLED_NUM_PINS] = { PinOwner::None }; // new MCU's have up to 50 GPIO
   #endif
   struct {
     uint8_t i2cAllocCount : 4; // allow multiple allocation of I2C bus pins but keep track of allocations