homieiot / homie-esp8266

💡 ESP8266 framework for Homie, a lightweight MQTT convention for the IoT
http://homieiot.github.io/homie-esp8266
MIT License
1.36k stars 308 forks source link

HomieSetting.setDefaultValue("") might be freed crashing the mcu #744

Closed empirephoenix closed 1 year ago

empirephoenix commented 1 year ago

Following a crash since ~1year I have the problem, that one of my projects (https://github.com/0110/PlantCtrl/tree/master/esp32) crash, when setting a char* setting, if it is not set, no crash occurs.

Following some debugging I think I have it narrowed down, and found the case to be related to https://github.com/homieiot/homie-esp8266/commit/71c3aaf1f5480b5f406fedb269bd09ec391c00f9

When using a const char setting, with a defaultValue set by code to "" eg. lipoSensorAddr.setDefaultValue(""); and then later the Config file is loaded, and set() is called with the saved value, it will in turn calls ffree(); this will (since that commit) execute free(const_cast<char>(_value)); After this the mcu crashes, as the variable static and probably compiled into the flash (and even if not, it would be on stack and out of scope already).

This does not happen without programatically calling the default value, as the mqqt based setter will use strdup() and thus will not attempt to free a part of the flash.

luebbe commented 1 year ago

So what is your proposed solution? Unfortunately this project has no active maintainer. I can merge PRs, but I don't have a deep enough understanding of the code and not enough time to work on bugfixes.

empirephoenix commented 1 year ago

I created a pull request for it, it should solve the issue.

luebbe commented 1 year ago

Merged, thanks.