_length member variable in WiFiManagerParameter object not initialized

[hocky101]

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.

[tablatronix]

hmm i thought I fixed that already, odd

[tablatronix]

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

[tablatronix]

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

[hocky101]

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.

[hansimglueck]

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...