[GH-ISSUE #1504] _length member variable in WiFiManagerParameter object not initialized #1284

Closed
opened 2026-02-28 01:29:24 +03:00 by kerem · 5 comments
Owner

Originally created by @hocky101 on GitHub (Sep 21, 2022).
Original GitHub issue: https://github.com/tzapu/WiFiManager/issues/1504

Basic Infos

Generic ESP32 DevKit Module

Hardware

WiFimanager Branch/Release: Master

version=2.0.13-beta

Esp8266/Esp32:

Generic ESP32 DevKit Module

Hardware: ESP32-WROOM-32

Core Version: 2.0.5

Description

_length member variable in WiFiManagerParameter object not initialized.

Problem description

With the following code:

WiFiManagerParameter custom_mqtt_server("server", "mqtt server", _mqtt_server, `40);

the constructor of WiFiManagerParameter() doesn't initialize the _length member variable.

WiFiManagerParameter::WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length) {
  init(id, label, defaultValue, length, "", WFM_LABEL_DEFAULT);
}

void WiFiManagerParameter::init(const char *id, const char *label, const char *defaultValue, int length, const char *custom, int labelPlacement) {
  _id             = id;
  _label          = label;
  _labelPlacement = labelPlacement;
  _customHTML     = custom;
  _value          = nullptr;
  setValue(defaultValue,length);
}

If it happened that the length parameter to the WiFiManagerParameter constructor is the same as _length member variable, then in setValue(), memory for _value would not be allocated, resulting in panic in the ensuing code.

void WiFiManagerParameter::setValue(const char *defaultValue, int length) {
  if(!_id){
    // Serial.println("cannot set value of this parameter");
    return;
  }
  
  // if(strlen(defaultValue) > length){
  //   // Serial.println("defaultValue length mismatch");
  //   // return false; //@todo bail 
  // }

  if(_length != length){
    _length = length;
    if( _value != nullptr){
      delete[] _value;
    }
    _value  = new char[_length + 1];  
  }

  memset(_value, 0, _length + 1); // explicit null
  
  if (defaultValue != NULL) {
    strncpy(_value, defaultValue, _length);
  }
}

In 2.0.12-beta, there is no comparison between _length member variable and length parameter, and _value is always allocated memory, which is why it doesn't crash in 2.0.12-beta.

Originally created by @hocky101 on GitHub (Sep 21, 2022). Original GitHub issue: https://github.com/tzapu/WiFiManager/issues/1504 ### Basic Infos Generic ESP32 DevKit Module #### Hardware WiFimanager Branch/Release: Master version=2.0.13-beta Esp8266/Esp32: Generic ESP32 DevKit Module Hardware: ESP32-WROOM-32 Core Version: 2.0.5 ### Description `_length` member variable in `WiFiManagerParameter` object not initialized. Problem description With the following code: ```C++ WiFiManagerParameter custom_mqtt_server("server", "mqtt server", _mqtt_server, `40); ``` the constructor of WiFiManagerParameter() doesn't initialize the _length member variable. ```C++ WiFiManagerParameter::WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length) { init(id, label, defaultValue, length, "", WFM_LABEL_DEFAULT); } void WiFiManagerParameter::init(const char *id, const char *label, const char *defaultValue, int length, const char *custom, int labelPlacement) { _id = id; _label = label; _labelPlacement = labelPlacement; _customHTML = custom; _value = nullptr; setValue(defaultValue,length); } ``` If it happened that the `length` parameter to the `WiFiManagerParameter` constructor is the same as `_length` member variable, then in `setValue()`, memory for `_value` would not be allocated, resulting in panic in the ensuing code. ``` C++ void WiFiManagerParameter::setValue(const char *defaultValue, int length) { if(!_id){ // Serial.println("cannot set value of this parameter"); return; } // if(strlen(defaultValue) > length){ // // Serial.println("defaultValue length mismatch"); // // return false; //@todo bail // } if(_length != length){ _length = length; if( _value != nullptr){ delete[] _value; } _value = new char[_length + 1]; } memset(_value, 0, _length + 1); // explicit null if (defaultValue != NULL) { strncpy(_value, defaultValue, _length); } } ``` In 2.0.12-beta, there is no comparison between `_length` member variable and `length` parameter, and `_value` is always allocated memory, which is why it doesn't crash in 2.0.12-beta.
kerem 2026-02-28 01:29:24 +03:00
  • closed this issue
  • added the
    bug
    label
Author
Owner

@tablatronix commented on GitHub (Sep 21, 2022):

hmm i thought I fixed that already, odd

<!-- gh-comment-id:1253934495 --> @tablatronix commented on GitHub (Sep 21, 2022): hmm i thought I fixed that already, odd
Author
Owner

@tablatronix commented on GitHub (Sep 21, 2022):

I cant get it to blow up at all, got an example or is this hypothetical?

Also are you getting a compiler warning, I would have expected one... hmm

<!-- gh-comment-id:1253970572 --> @tablatronix commented on GitHub (Sep 21, 2022): I cant get it to blow up at all, got an example or is this hypothetical? Also are you getting a compiler warning, I would have expected one... hmm
Author
Owner

@tablatronix commented on GitHub (Sep 21, 2022):

i cant think of a way to reliable unit test this oh well, I dont think will be a major issue needing a patch

<!-- gh-comment-id:1253979917 --> @tablatronix commented on GitHub (Sep 21, 2022): i cant think of a way to reliable unit test this oh well, I dont think will be a major issue needing a patch
Author
Owner

@hocky101 commented on GitHub (Sep 22, 2022):

It is not hypothetical. I found it on my existing projects. I didn't include the code as it is a large sketch consisting of multiple files & libraries.

Following is an example code to reproduce the issue.

#include <WiFiManager.h>

class EspWiFiApplication
{
private:
  char  _mqtt_server[40];
  char  _ntp_server[40];

public:
  void initialize(int count = 0, /* WiFiManagerParameters* */...);
  
};

void EspWiFiApplication::initialize(int count, /* WiFiManagerParameters* */...)
{
  WiFiManagerParameter custom_mqtt_server("mqtt", "mqtt server", _mqtt_server, 40);
  WiFiManagerParameter custom_ntp_server("ntp", "nyp server", _ntp_server, 40);

  static WiFiManager wifiManager;

  wifiManager.addParameter(&custom_mqtt_server);
  wifiManager.addParameter(&custom_ntp_server);
}


EspWiFiApplication app;

char  _host_name[40];

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  Serial.println("BUG1504 TEST");

  WiFiManagerParameter custom_host_name("host", "host name", _host_name, 40);

  app.initialize(1, &custom_host_name);
}

void loop() {
  // put your main code here, to run repeatedly:

}

And the debug output with changes in WiFiManagerParameter::init() to print out the length & _length values.

09:59:17.520 -> BUG1504 TEST
09:59:17.520 -> length = 40, _length = 0
09:59:17.520 -> length = 40, _length = 40
10:04:46.110 -> Guru Meditation Error: Core  1 panic'ed (StoreProhibited). Exception was unhandled.

It panic'd on the 2nd WiFiManagerParameter initialization:

  WiFiManagerParameter custom_mqtt_server("mqtt", "mqtt server", _mqtt_server, 40);

Hope this help.

With your latest changes _length = 1 in WiFiManager.cpp, the bug is fixed. Thanks.

<!-- gh-comment-id:1254422654 --> @hocky101 commented on GitHub (Sep 22, 2022): It is not hypothetical. I found it on my existing projects. I didn't include the code as it is a large sketch consisting of multiple files & libraries. Following is an example code to reproduce the issue. ```C++ #include <WiFiManager.h> class EspWiFiApplication { private: char _mqtt_server[40]; char _ntp_server[40]; public: void initialize(int count = 0, /* WiFiManagerParameters* */...); }; void EspWiFiApplication::initialize(int count, /* WiFiManagerParameters* */...) { WiFiManagerParameter custom_mqtt_server("mqtt", "mqtt server", _mqtt_server, 40); WiFiManagerParameter custom_ntp_server("ntp", "nyp server", _ntp_server, 40); static WiFiManager wifiManager; wifiManager.addParameter(&custom_mqtt_server); wifiManager.addParameter(&custom_ntp_server); } EspWiFiApplication app; char _host_name[40]; void setup() { // put your setup code here, to run once: Serial.begin(115200); Serial.println("BUG1504 TEST"); WiFiManagerParameter custom_host_name("host", "host name", _host_name, 40); app.initialize(1, &custom_host_name); } void loop() { // put your main code here, to run repeatedly: } ``` And the debug output with changes in `WiFiManagerParameter::init()` to print out the `length` & `_length` values. ``` 09:59:17.520 -> BUG1504 TEST 09:59:17.520 -> length = 40, _length = 0 09:59:17.520 -> length = 40, _length = 40 10:04:46.110 -> Guru Meditation Error: Core 1 panic'ed (StoreProhibited). Exception was unhandled. ``` It panic'd on the 2nd `WiFiManagerParameter` initialization: ``` WiFiManagerParameter custom_mqtt_server("mqtt", "mqtt server", _mqtt_server, 40); ``` Hope this help. With your latest changes `_length = 1` in `WiFiManager.cpp`, the bug is fixed. Thanks.
Author
Owner

@hansimglueck commented on GitHub (Nov 23, 2022):

Hi. Even though I dont really get what this is about, I found that when I have a length = 1 this leads to kernel panic now...
WiFiManagerParameter modeParam("mode", "Mode - 0: normal, 1: special", "0", 1);

At least after reading this thread I just changed the length to 2 and its working again...

<!-- gh-comment-id:1324430561 --> @hansimglueck commented on GitHub (Nov 23, 2022): Hi. Even though I dont really get what this is about, I found that when I have a length = 1 this leads to kernel panic now... `WiFiManagerParameter modeParam("mode", "Mode - 0: normal, 1: special", "0", 1);` At least after reading this thread I just changed the length to 2 and its working again...
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/WiFiManager#1284
No description provided.