[GH-ISSUE #1341] WiFiManagerParameter does not handle move constructor #1150

New issue

Open

opened 2026-02-28 01:28:45 +03:00 by kerem · 4 comments

kerem commented 2026-02-28 01:28:45 +03:00

Owner

Copy link

Originally created by @axlan on GitHub (Jan 22, 2022).
Original GitHub issue: https://github.com/tzapu/WiFiManager/issues/1341

Hardware

WiFimanager Branch/Release: Master commit 810f144

Esp8266/Esp32:

Hardware: D1-mini lite

Description

WiFiManagerParameter class does not properly handle the C++ move constructor resulting in a use after free error.

Reproduction Example

#include <vector>

#include <WiFiManager.h> // https://github.com/tzapu/WiFiManager

WiFiManager wm;
std::vector<WiFiManagerParameter> parameters;

void setup() {
    WiFi.mode(WIFI_STA); // explicitly set mode, esp defaults to STA+AP    
    // put your setup code here, to run once:
    Serial.begin(115200);

    // Allocate new space and construct parameter in it
    parameters.emplace_back("id1", "field1", "", 40);
    // Allocate new space, move "id1" and construct parameter after it
    parameters.emplace_back("id2", "field2", "", 40);

    // Add parameters to WiFiManager
    for (auto& param : parameters)
    {
      wm.addParameter(&param);
    }

    wm.setConfigPortalBlocking(false);
    //automatically connect using saved credentials if they exist
    //If connection fails it starts an access point with the specified name
    if(wm.autoConnect("AutoConnectAP")){
        Serial.println("connected...yeey :)");
    }
    else {
        Serial.println("Configportal running");
    }

    wm.startConfigPortal();
}

void loop() {
    wm.process();
}

I added a log message to record the alloc/free's:

new char allocated id1
new char allocated id2
char freed id1
*wm:[2] Added Parameter: id1
*wm:[2] Added Parameter: id2

Solution

What's happening here is that the std::vector is calling the implicit move constructor: https://en.cppreference.com/w/cpp/language/move_constructor

This constructor doesn't correctly handle moving the _value data which is then freed. A similar issue is presumably why the copy constructor is made private.

There are three fixes that I see.

1. Delete the move constructor. My example doesn't build if I add: WiFiManagerParameter(const WiFiManagerParameter&&) = delete;
2. Implement a move (and optionally a copy) constructor. This is fairly straightforward and just needs to correctly propagate the pointer to the new instance and set the old instance to NULL so it isn't freed.
3. Switch to using smart pointers. I took this approach for a fork I made for testing. Here's the code:

class WiFiManagerParameter {
  public:
    /** 
        Create custom parameters that can be added to the WiFiManager setup web page
        @id is used for HTTP queries and must not contain spaces nor other special characters
    */
    WiFiManagerParameter() = default;
    WiFiManagerParameter(const char *custom);
    WiFiManagerParameter(const char *id, const char *label);
    WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length);
    WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length, const char *custom);
    WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length, const char *custom, int labelPlacement);

    const char *getID() const;
    const char *getValue() const;
    const char *getLabel() const;
    const char *getPlaceholder() const; // @deprecated, use getLabel
    int         getValueLength() const;
    int         getLabelPlacement() const;
    virtual const char *getCustomHTML() const;
    void        setValue(const char *defaultValue, int length);

  protected:
    void init(const char *id, const char *label, const char *defaultValue, int length, const char *custom, int labelPlacement);

  private:
    const char *_id = nullptr;
    const char *_label = nullptr;
    std::unique_ptr<char[]> _value;
    int         _length = 1;
    int         _labelPlacement = WFM_LABEL_BEFORE;
  protected:
    const char *_customHTML = "";
    friend class WiFiManager;
};


WiFiManagerParameter::WiFiManagerParameter(const char *custom) {
  _customHTML     = custom;
}

WiFiManagerParameter::WiFiManagerParameter(const char *id, const char *label) {
  init(id, label, "", 0, "", WFM_LABEL_BEFORE);
}

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

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

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

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;
  setValue(defaultValue,length);
}

// WiFiManagerParameter& WiFiManagerParameter::operator=(const WiFiManagerParameter& rhs){
//   Serial.println("copy assignment op called");
//   (*this->_value) = (*rhs._value);
//   return *this;
// }

// @note debug is not available in wmparameter class
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 
  // }

  _length = length;
  _value  = std::make_unique<char[]>(_length + 1);
  memset(_value.get(), 0, _length + 1); // explicit null
  
  if (defaultValue != NULL) {
    strncpy(_value.get(), defaultValue, _length);
  }
}
const char* WiFiManagerParameter::getValue() const {
  // Serial.println(printf("Address of _value is %p\n", (void *)_value)); 
  return _value.get();
}
const char* WiFiManagerParameter::getID() const {
  return _id;
}
const char* WiFiManagerParameter::getPlaceholder() const {
  return _label;
}
const char* WiFiManagerParameter::getLabel() const {
  return _label;
}
int WiFiManagerParameter::getValueLength() const {
  return _length;
}
int WiFiManagerParameter::getLabelPlacement() const {
  return _labelPlacement;
}
const char* WiFiManagerParameter::getCustomHTML() const {
  return _customHTML;
}

Note: the copy constructor is implicitly deleted by the inclusion of the unique_ptr.

A simple copy constructor could be implemented with:

WiFiManagerParameter::WiFiManagerParameter(const WiFiManagerParameter& other) {
  Serial.println("copy assignment op called");
  _id             = other._id;
  _label          = other._label;
  _labelPlacement = other._labelPlacement;
  _customHTML     = other._customHTML;
  setValue(other.getValue(), other._length);
}

The only change to the rest of the project was replacing the line:

value.toCharArray(_params[i]->_value, _params[i]->_length+1); // length+1 null terminated
with
value.toCharArray(_params[i]->_value.get(), _params[i]->_length+1); // length+1 null terminated

Originally created by @axlan on GitHub (Jan 22, 2022). Original GitHub issue: https://github.com/tzapu/WiFiManager/issues/1341 #### Hardware WiFimanager Branch/Release: Master commit 810f144 Esp8266/Esp32: Hardware: D1-mini lite ### Description WiFiManagerParameter class does not properly handle the C++ move constructor resulting in a use after free error. ### Reproduction Example ```cpp #include <vector> #include <WiFiManager.h> // https://github.com/tzapu/WiFiManager WiFiManager wm; std::vector<WiFiManagerParameter> parameters; void setup() { WiFi.mode(WIFI_STA); // explicitly set mode, esp defaults to STA+AP // put your setup code here, to run once: Serial.begin(115200); // Allocate new space and construct parameter in it parameters.emplace_back("id1", "field1", "", 40); // Allocate new space, move "id1" and construct parameter after it parameters.emplace_back("id2", "field2", "", 40); // Add parameters to WiFiManager for (auto& param : parameters) { wm.addParameter(&param); } wm.setConfigPortalBlocking(false); //automatically connect using saved credentials if they exist //If connection fails it starts an access point with the specified name if(wm.autoConnect("AutoConnectAP")){ Serial.println("connected...yeey :)"); } else { Serial.println("Configportal running"); } wm.startConfigPortal(); } void loop() { wm.process(); } ``` I added a log message to record the alloc/free's: ``` new char allocated id1 new char allocated id2 char freed id1 *wm:[2] Added Parameter: id1 *wm:[2] Added Parameter: id2 ``` ### Solution What's happening here is that the `std::vector` is calling the implicit move constructor: https://en.cppreference.com/w/cpp/language/move_constructor This constructor doesn't correctly handle moving the `_value` data which is then freed. A similar issue is presumably why the copy constructor is made private. There are three fixes that I see. 1. Delete the move constructor. My example doesn't build if I add: `WiFiManagerParameter(const WiFiManagerParameter&&) = delete;` 2. Implement a move (and optionally a copy) constructor. This is fairly straightforward and just needs to correctly propagate the pointer to the new instance and set the old instance to NULL so it isn't freed. 3. Switch to using smart pointers. I took this approach for a fork I made for testing. Here's the code: ```cpp class WiFiManagerParameter { public: /** Create custom parameters that can be added to the WiFiManager setup web page @id is used for HTTP queries and must not contain spaces nor other special characters */ WiFiManagerParameter() = default; WiFiManagerParameter(const char *custom); WiFiManagerParameter(const char *id, const char *label); WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length); WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length, const char *custom); WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length, const char *custom, int labelPlacement); const char *getID() const; const char *getValue() const; const char *getLabel() const; const char *getPlaceholder() const; // @deprecated, use getLabel int getValueLength() const; int getLabelPlacement() const; virtual const char *getCustomHTML() const; void setValue(const char *defaultValue, int length); protected: void init(const char *id, const char *label, const char *defaultValue, int length, const char *custom, int labelPlacement); private: const char *_id = nullptr; const char *_label = nullptr; std::unique_ptr<char[]> _value; int _length = 1; int _labelPlacement = WFM_LABEL_BEFORE; protected: const char *_customHTML = ""; friend class WiFiManager; }; WiFiManagerParameter::WiFiManagerParameter(const char *custom) { _customHTML = custom; } WiFiManagerParameter::WiFiManagerParameter(const char *id, const char *label) { init(id, label, "", 0, "", WFM_LABEL_BEFORE); } WiFiManagerParameter::WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length) { init(id, label, defaultValue, length, "", WFM_LABEL_BEFORE); } WiFiManagerParameter::WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length, const char *custom) { init(id, label, defaultValue, length, custom, WFM_LABEL_BEFORE); } WiFiManagerParameter::WiFiManagerParameter(const char *id, const char *label, const char *defaultValue, int length, const char *custom, int labelPlacement) { init(id, label, defaultValue, length, custom, labelPlacement); } 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; setValue(defaultValue,length); } // WiFiManagerParameter& WiFiManagerParameter::operator=(const WiFiManagerParameter& rhs){ // Serial.println("copy assignment op called"); // (*this->_value) = (*rhs._value); // return *this; // } // @note debug is not available in wmparameter class 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 // } _length = length; _value = std::make_unique<char[]>(_length + 1); memset(_value.get(), 0, _length + 1); // explicit null if (defaultValue != NULL) { strncpy(_value.get(), defaultValue, _length); } } const char* WiFiManagerParameter::getValue() const { // Serial.println(printf("Address of _value is %p\n", (void *)_value)); return _value.get(); } const char* WiFiManagerParameter::getID() const { return _id; } const char* WiFiManagerParameter::getPlaceholder() const { return _label; } const char* WiFiManagerParameter::getLabel() const { return _label; } int WiFiManagerParameter::getValueLength() const { return _length; } int WiFiManagerParameter::getLabelPlacement() const { return _labelPlacement; } const char* WiFiManagerParameter::getCustomHTML() const { return _customHTML; } ``` Note: the copy constructor is implicitly deleted by the inclusion of the unique_ptr. A simple copy constructor could be implemented with: ```cpp WiFiManagerParameter::WiFiManagerParameter(const WiFiManagerParameter& other) { Serial.println("copy assignment op called"); _id = other._id; _label = other._label; _labelPlacement = other._labelPlacement; _customHTML = other._customHTML; setValue(other.getValue(), other._length); } ``` The only change to the rest of the project was replacing the line: `value.toCharArray(_params[i]->_value, _params[i]->_length+1); // length+1 null terminated` with `value.toCharArray(_params[i]->_value.get(), _params[i]->_length+1); // length+1 null terminated`

kerem added the

enhancement

Branch

labels 2026-02-28 01:28:45 +03:00

kerem commented 2026-02-28 01:28:45 +03:00

Author

Owner

Copy link

@tablatronix commented on GitHub (Jan 22, 2022):

I see no issues switching to smart pointers, I might just need someone else to review this

<!-- gh-comment-id:1019363136 --> @tablatronix commented on GitHub (Jan 22, 2022): I see no issues switching to smart pointers, I might just need someone else to review this

kerem commented 2026-02-28 01:28:45 +03:00

Author

Owner

Copy link

@tablatronix commented on GitHub (Jan 22, 2022):

here is the issue for that change to copy operator

https://github.com/tzapu/WiFiManager/issues/1050

<!-- gh-comment-id:1019363866 --> @tablatronix commented on GitHub (Jan 22, 2022): here is the issue for that change to copy operator https://github.com/tzapu/WiFiManager/issues/1050

kerem commented 2026-02-28 01:28:45 +03:00

Author

Owner

Copy link

@tablatronix commented on GitHub (Feb 19, 2022):

FYI this is applied to branch
https://github.com/tzapu/WiFiManager/tree/parameter-smart-ptr

anyone test this?

<!-- gh-comment-id:1046077183 --> @tablatronix commented on GitHub (Feb 19, 2022): FYI this is applied to branch https://github.com/tzapu/WiFiManager/tree/parameter-smart-ptr anyone test this?

kerem commented 2026-02-28 01:28:45 +03:00

Author

Owner

Copy link

@tablatronix commented on GitHub (Apr 7, 2022):

BUMP

<!-- gh-comment-id:1091775404 --> @tablatronix commented on GitHub (Apr 7, 2022): BUMP

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

master

revert-1849-master

feature_fastconnect

feature_auth

feature_slimable

PEAP

feature_statichtmlparams

feature_asyncwebserver

feature_pagestringsref

parameter-smart-ptr

gh-pages

Legacy_v1

hotfixes

autochannelselect_feature

travis

smartconfig

v2.0.17

v2.0.16-rc.2

v2.0.15-rc.1

v2.0.0-rc.1

v2.0.14-beta

v2.0.13-beta

v2.0.12-beta

v2.0.11-beta

v2.0.10-beta

v2.0.9-beta

v2.0.8-beta

2.0.7-beta

2.0.6-beta

2.0.5-beta

2.0.4-beta

0.16.0

2.0.3-alpha

0.15.0

0.14

0.13

0.12

0.11

0.10

0.9

0.8

0.7

0.6

0.5

v0.4

v0.3

v0.2

v0.1

Labels

Clear labels   

📶 WiFi

  

🕸️ HTTP

  

Branch

  

DEV Help Wanted

  

Discussion

  

Documentation

  

ESP32

  

Example

  

Good First Issue

  

Hotfix

  

In Progress

  

Incomplete

  

Needs Feeback

  

Priority

  

QA

  

Question

  

Task

  

Upstream/Dependancy

  

bug

  

duplicate

  

enhancement

  

invalid

  

pull-request

Mirrored from GitHub Pull Request

  

wontfix

No labels

📶 WiFi

🕸️ HTTP

Branch

DEV Help Wanted

Discussion

Documentation

ESP32

Example

Good First Issue

Hotfix

In Progress

Incomplete

Needs Feeback

Priority

QA

Question

Task

Upstream/Dependancy

bug

duplicate

enhancement

invalid

pull-request

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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#1150

No description provided.