Skip to content

Fix use-after-free when temperature sensors are re-initialised after a config save - #1263

Open
Allram wants to merge 1 commit into
UtilitechAS:mainfrom
Allram:pr/temp-sensor-use-after-free
Open

Allram wants to merge 1 commit into
UtilitechAS:mainfrom
Allram:pr/temp-sensor-use-after-free

Conversation

@Allram

@Allram Allram commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Problem

HwTools::setup() runs after every web UI save that does not require a restart (see AmsWebServer::handleSave), and it clears tempSensorInit. The next updateTemperatures() therefore enters the init branch again while sensorCount is still set from the previous run. That branch deleted the tempSensors array and then searched it for matching addresses, dereferencing freed memory.

On ESP8266 this reproduces as an exception 28 (LoadProhibited) in isSensorAddressEqual() on the first temperature read after saving, for example after changing MQTT settings on a device with a DS18B20 attached. Stack from the crash:

epc1=0x4024f972 excvaddr=0x000006c7
HwTools::isSensorAddressEqual -> handleTemperature -> loop

Fix

Keep the old array alive while matching addresses, move surviving entries into the new array, free entries for sensors that are gone, and use delete[] for the array.

Tested

ESP8266 (D1 mini), one DS18B20 on GPIO14, v2.5.7 and current main. Saving MQTT settings crashed the device every time before; after the fix the save completes and the sensor keeps reporting.

…a config save

HwTools::setup() runs after every web UI save that does not require a
restart, and it clears tempSensorInit. The next updateTemperatures() then
enters the init branch again with sensorCount still set from the previous
run: it deletes the tempSensors array and immediately searches it for
matching addresses, dereferencing freed memory.

On ESP8266 this shows up as an exception 28 (LoadProhibited) in
isSensorAddressEqual() on the first temperature read after saving, e.g.
changing MQTT settings on a device with a DS18B20 attached.

Keep the old array alive while matching, move surviving entries into the
new array, and free the rest. Also use delete[] for the array.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant