Crash in WiFiScanClass::_scanDone() with negative scan result

[TD-er]

Board

Any

Device Description

Not related to device, but crash happens on ESP32-C2 and -C3

Hardware Configuration

Not HW related

Version

latest master (checkout manually)

IDE Name

PlatformIO

Operating System

Windows 11

Flash frequency

40MHz

PSRAM enabled

no

Upload speed

115200

Description

I get crashes related to the new-operator when processing results from a WiFi scan
It seems to be caused by the code in this function:

arduino-esp32/libraries/WiFi/src/WiFiScan.cpp

Lines 109 to 121 in b811ea4

	void WiFiScanClass::_scanDone()
	{
	esp_wifi_scan_get_ap_num(&(WiFiScanClass::_scanCount));
	if(WiFiScanClass::_scanCount) {
	WiFiScanClass::_scanResult = new wifi_ap_record_t[WiFiScanClass::_scanCount];
	if(!WiFiScanClass::_scanResult || esp_wifi_scan_get_ap_records(&(WiFiScanClass::_scanCount), (wifi_ap_record_t*)_scanResult) != ESP_OK) {
	WiFiScanClass::_scanCount = 0;
	}
	}
	WiFiScanClass::_scanStarted=0; //Reset after a scan is completed for normal behavior
	WiFiGenericClass::setStatusBits(WIFI_SCAN_DONE_BIT);
	WiFiGenericClass::clearStatusBits(WIFI_SCANNING_BIT);
	}

As can be seen, there is no check for negative scan results as the type of _scanCount is an uint16_t.
However the result of a scan can be negative, so maybe there is some conversion somewhere to this unsigned value and thus resulting in an attempt to allocate 65k elements of wifi_ap_record_t, which does fail at least on a C2.

Sketch

-

Debug Message

abort() was called at PC 0x420e944d on core 0
=> 0x420e944d: __wrap__Unwind_RaiseException at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/cxx/cxx_exception_stubs.cpp:156
Core  0 register dump:
MEPC    : 0x40381ce8  RA      : 0x40385308  SP      : 0x3fcbb380  GP      : 0x3fca8e60
=> 0x40381ce8: panic_abort at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/esp_system/panic.c:471
=> 0x40385308: __ubsan_include at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/esp_system/ubsan.c:313
TP      : 0x3fc92268  T0      : 0x37363534  T1      : 0x7271706f  T2      : 0x33323130
S0/FP   : 0x3fcbb3ac  S1      : 0x3fcbb390  A0      : 0x3fcbb3ac  A1      : 0x3fcbb38e
A2      : 0x00000000  A3      : 0x3fcbb3d9  A4      : 0x00000001  A5      : 0x3fcb3000
A6      : 0x00000000  A7      : 0x76757473  S2      : 0x3fcab6d0  S3      : 0x3fcab810
S4      : 0x3fcb3000  S5      : 0x00000029  S6      : 0x00000000  S7      : 0x00000000
S8      : 0x00000000  S9      : 0x00000000  S10     : 0x00000000  S11     : 0x00000000
T3      : 0x6e6d6c6b  T4      : 0x6a696867  T5      : 0x66656463  T6      : 0x62613938
MSTATUS : 0x00001801  MTVEC   : 0x40380001  MCAUSE  : 0x00000007  MTVAL   : 0x00000000
=> 0x40380001: _vector_table at ??:?
MHARTID : 0x00000000

Stack memory:
3fcbb380: 0x3fcab6d0 0x3fcc132c 0x3fcb3b10 0x40380030 0x65303234 0x64343439 0x3fcb3000 0x3fcab0b0
=> 0x40380030: _vector_table at ??:?
3fcbb3a0: 0x3fcbb390 0x3fcab0cc 0x3fcbb38c 0x726f6261 0x20292874 0x20736177 0x6c6c6163 0x61206465
3fcbb3c0: 0x43502074 0x34783020 0x39653032 0x20643434 0x63206e6f 0x2065726f 0x00000030 0x420e0000
=> 0x43502074: ?? ??:0
=> 0x420e0000: cnx_do_handoff at wl_cnx.o:?
3fcbb3e0: 0x3fcab6d0 0x3fcc1284 0x3fcc138c 0x420e9450 0x3fcab6d0 0x3fcc1284 0x420e8e68 0x3c12774c
=> 0x420e9450: __wrap___gxx_personality_v0 at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/cxx/cxx_exception_stubs.cpp:33
      (inlined by) __wrap___gxx_personality_v0 at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/cxx/cxx_exception_stubs.cpp:185
=> 0x420e8e68: std::bad_alloc::~bad_alloc() at /builds/idf/crosstool-NG/.build/HOST-x86_64-w64-mingw32/riscv32-esp-elf/src/gcc/libstdc++-v3/libsupc++/bad_alloc.cc:28
3fcbb400: 0x3fcab6d0 0x3fcc1284 0x00000498 0x420e856e 0x3fcab6d0 0x3fcc046c 0x3fcb3000 0x420073ee
=> 0x420e856e: operator new(unsigned int) at /builds/idf/crosstool-NG/.build/HOST-x86_64-w64-mingw32/riscv32-esp-elf/src/gcc/libstdc++-v3/libsupc++/new_op.cc:55
=> 0x420073ee: WiFiScanClass::_scanDone() at C:/Users/gijsn/.platformio/packages/framework-arduinoespressif32/libraries/WiFi/src/WiFiScan.cpp:113
      (inlined by) WiFiGenericClass::_eventCallback(arduino_event_t*) at C:/Users/gijsn/.platformio/packages/framework-arduinoespressif32/libraries/WiFi/src/WiFiGeneric.cpp:1044
      (inlined by) _arduino_event_task at C:/Users/gijsn/.platformio/packages/framework-arduinoespressif32/libraries/WiFi/src/WiFiGeneric.cpp:307
3fcbb420: 0xa5a5a5a5 0xa5a5a5a5 0xa5a5a5a5 0xa5a5a5a5 0xa5a5a5a5 0x3fcc1284 0x3c101d88 0x00000000
3fcbb440: 0x00000000 0x00000000 0x00000000 0xa5a5a500 0xa5a5a500 0x3c101d88 0x00000000 0x00000000
3fcbb460: 0x00000000 0x00000000 0xa5a5a500 0xa5a5a500 0x00000002 0x00000000 0x00000000 0x00000000
3fcbb480: 0x00000000 0xa5a5a500 0xa5a5a500 0x00000010 0x00000000 0x4203a166 0x00000000 0x420388da
=> 0x4203a166: WiFiEvent(arduino_event_id_t, arduino_event_info_t) at src/src/ESPEasyCore/ESPEasyWiFiEvent.cpp:61
=> 0x420388da: std::_Function_handler<void (arduino_event_id_t, arduino_event_info_t), void (*)(arduino_event_id_t, arduino_event_info_t)>::_M_manager(std::_Any_data&, std::_Any_data const&, std::_Manager_operation) at c:\users\gijsn\.platformio\packages\toolchain-riscv32-esp\riscv32-esp-elf\include\c++\12.2.0\bits/std_function.h:267
3fcbb4a0: 0x420388aa 0x00000000 0x00000029 0x3fca8e60 0x3fc91258 0x4038553e 0x40385a14 0xffffffff
=> 0x420388aa: std::_Function_handler<void (arduino_event_id_t, arduino_event_info_t), void (*)(arduino_event_id_t, arduino_event_info_t)>::_M_invoke(std::_Any_data const&, arduino_event_id_t&&, arduino_event_info_t&&) at c:\users\gijsn\.platformio\packages\toolchain-riscv32-esp\riscv32-esp-elf\include\c++\12.2.0\bits/std_function.h:288
=> 0x4038553e: vPortEnterCritical at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/FreeRTOS-Kernel/portable/riscv/port.c:323
=> 0x40385a14: xTaskIncrementTick at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/FreeRTOS-Kernel/tasks.c:3345
      (inlined by) xTaskIncrementTick at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/FreeRTOS-Kernel/tasks.c:3294
3fcbb4c0: 0x3fcb6e04 0x00000000 0x00000001 0x00000001 0x00000014 0x00000004 0x00000001 0x600c0000
3fcbb4e0: 0x00000010 0x00000000 0x3fcba510 0xffffffff 0xffffffff 0x00000000 0x00000000 0x00000000
3fcbb500: 0xffffffff 0x00000000 0x3fcb7068 0x420f7d28 0x420f7e72 0x3fcb7068 0x00000000 0xffffffff
=> 0x420f7d28: xQueueSemaphoreTake at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/FreeRTOS-Kernel/queue.c:1675
=> 0x420f7e72: xQueueTakeMutexRecursive at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/FreeRTOS-Kernel/queue.c:788
3fcbb520: 0x3fcba510 0x00000000 0x3fcb6e04 0x420f7cd6 0x00000000 0x00000000 0xffffffff 0xffffffff
=> 0x420f7cd6: xQueueReceive at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/FreeRTOS-Kernel/queue.c:1495
3fcbb540: 0x00000000 0x00000000 0x3fcb7068 0x420f7e72 0x00000000 0x00000000 0xffffffff 0xffffffff
=> 0x420f7e72: xQueueTakeMutexRecursive at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/FreeRTOS-Kernel/queue.c:788
3fcbb560: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000
3fcbb580: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0xa5a5a5a5 0xa5a5a5a5 0xa5a5a5a5
3fcbb5a0: 0xa5a5a5a5 0xa5a5a5a5 0xbaad5678 0x00000160 0xabba1234 0x00000154 0x3fcbb350 0x0015fd1b
3fcbb5c0: 0x3fcae284 0x3fcae284 0x3fcbb5b8 0x3fcae27c 0x00000006 0x3fcb6d24 0x3fcb6d24 0x3fcbb5b8
3fcbb5e0: 0x00000000 0x00000013 0x3fcba5a8 0x75647261 0x5f6f6e69 0x6e657665 0x00007374 0x00000000
3fcbb600: 0x3fcbb5a0 0x00000013 0x00000000 0x3fcc2130 0x4208e272 0x00000000 0x3fcb371c 0x3fcb3784
=> 0x4208e272: pthread_cleanup_thread_specific_data_callback at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/pthread/pthread_local_storage.c:126
3fcbb620: 0x3fcb37ec 0x00000000 0x00000000 0x00000001 0x00000000 0x00000000 0x00000000 0x420f0e7a
=> 0x420f0e7a: _cleanup_r at /builds/idf/crosstool-NG/.build/HOST-x86_64-w64-mingw32/riscv32-esp-elf/src/newlib/newlib/libc/stdio/findfp.c:229
3fcbb640: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000
3fcbb660: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000
3fcbb680: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000
3fcbb6a0: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000
3fcbb6c0: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000
3fcbb6e0: 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000
3fcbb700: 0x00000000 0x00000000 0x1a000000 0xbaad5678 0x0000001c 0xabba1234 0x00000010 0x3c126548
3fcbb720: 0x3fcbb73c 0x00000000 0x3fcbb76c 0xbaad5678 0x00000014 0xabba1234 0x00000008 0x3fcbb754
3fcbb740: 0x3fcc0850 0xbaad5678 0x00000014 0xabba1234 0x00000008 0x4200154c 0x00000000 0xbaad5678
=> 0x4200154c: _arduino_event_cb(void*, char const*, int, void*) at C:/Users/gijsn/.platformio/packages/framework-arduinoespressif32/libraries/WiFi/src/WiFiGeneric.cpp:334
3fcbb760: 0x0000001c 0xabba1234 0x00000010 0x3c126558 0x3fcbb78c 0x00000000 0x3fcceb50 0xbaad5678

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

• I confirm I have checked existing issues, online documentation and Troubleshooting guide.

[TD-er]

I'm looking into this code a bit more and there are lots of really strange things here.

For example:

void * WiFiScanClass::_getScanInfoByIndex(int i)
{
    if(!WiFiScanClass::_scanResult || (size_t) i >= WiFiScanClass::_scanCount) {
        return 0;
    }
    return reinterpret_cast<wifi_ap_record_t*>(WiFiScanClass::_scanResult) + i;
}

A very tricky way to not check for i < 0.

One of the return statements in WiFiScanClass::scanNetworks(...):

        if(WiFiGenericClass::waitStatusBits(WIFI_SCAN_DONE_BIT, 10000)){
            return (int16_t) WiFiScanClass::_scanCount;
        }

So this clearly shows it should be considered a signed int, as does int16_t WiFiScanClass::scanComplete()

I can't find what has been changed recently, so no idea why it is now completely unusable where it was running fine on previous ESP-IDF5.1 builds I made.
The code I mentioned here doesn't seem to have changed in years.

Edit:
Forgot to mention, but it might also be a memory leak, as there is no check to see if the array _scanResult wasn't already allocated.

[TD-er]

Not sure if it can be used on array allocations, but isn't the simplest fix to just do new (std::nothrow) here?

[P-R-O-C-H-Y]

@SuGlider Can you please take a look as you are now investigation WiFi changes? Thanks

[SuGlider]

@TD-er - Please provide a sketch that I can use to reproduce this issue.

[SuGlider]

It is not clear to me how and why you need to use _scanDone() in your sketch.

This fuction will be called from the WiFi Event Callback only when it gets ARDUINO_EVENT_WIFI_SCAN_DONE.
Therefore, the number _scanResult is positive or zero. Never negative.

[TD-er]

It is being called when the scan is done. I can't not call it.

It's called from:

esp_err_t WiFiGenericClass::_eventCallback(arduino_event_t *event)

with event ID ARDUINO_EVENT_WIFI_SCAN_DONE

[SuGlider]

Sorry, you replied fast... :-) -- I was editing my reply - including exactly what you wrote.
Anyway I can't reproduce the issue.

[SuGlider]

I need some sketch that I can use to reproduce this issue and find out what could be its cause.

[TD-er]

This fuction will be called from the WiFi Event Callback only when it gets ARDUINO_EVENT_WIFI_SCAN_DONE.
Therefore, the number _scanResult is positive or zero. Never negative.

But what if the scan is called again while busy or failed?
Then it is assigned a negative value, stored in an unsigned int, thus the argument for the new call can request a really large array.
On the ESP32-C2 this is very likely to fail due to its more limited amount of memory.

I have to think about some short sketch to reproduce, as you probably don't think "the entire ESPEasy project" is "a short sketch" ;)

[SuGlider]

In my opinion, _scanDone() should never be called by the user side (sketch). It is meant to be called only when the WiFi Event Handler needs to fill up the list of scanned SSIDs.

IF the sketch calls it, the code must make sure that the previous array was deleted, freeing the memory, and that a previous scan process has succeeded.