httpUpdateSigned crashes on verification when the digital signature does not match

[JiriBilek]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• [commit from Feb-28] I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• [n/a] If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: [ESP-12F]
• Core Version: [Feb-28-2020]
• Development Env: [Arduino IDE or Sloeber]
• Operating System: [Windows 10]

Settings in IDE

• Module: [Generic ESP8266 Module]
• Flash Mode: [qio]
• Flash Size: [4MB, FS:1MB, OTA:1019KB]
• lwip Variant: [v2 Lower Memory]
• Reset Method: [no dtr (aka ck)]
• Flash Frequency: [40Mhz]
• CPU Frequency: [80Mhz]
• Upload Using: [OTA]
• Upload Speed: [n/a] (serial upload only)

Problem Description

The OTA update with default signature verification (MANUAL_SIGNING = 0) work fine up to the moment when I tamper the signature in the bin file.
Any byte changed in the 256 byte signature will cause wdt or various crashes. Changing the last 4 bytes (the signature length) does not crash the CPU, it correctly throws an error and resumes the sketch.
I traced the problem up to the file Updater.cpp. Changing the line

Arduino/cores/esp8266/Updater.cpp

Line 264 in 845241b

if (!_verify->verify(_hash, (void *)sig, sigLen)) {

to e.g. if (sig[0xf0] == 0) { will allow playing with the signature and not crashing the CPU. Of course, it removes the digital signature verification :), it is only for tracing the bug.

I am asking for help as I don't understand the RSA stuff in

Arduino/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp

Line 873 in 845241b

bool SigningVerifier::verify(UpdaterHashClass *hash, const void *signature, uint32_t signatureLen) {

(the function used for verifying the signature).

Thanks for your time.

MCVE Sketch

Standard example Arduino/libraries/ESP8266httpUpdate/examples/httpUpdateSecure/httpUpdateSecure.ino, no code changes, only RSA keys changed, generated by OpenSSL. I can upload them if it's worth doing it.

Debug Messages

wdt or Fatal exception 2(InstructionRetchErrorCause) at various addresses (epc1)
I will decode the stack and update the post shortly.

Edit: the dump

Exception 2: InstructionFetchError: Processor internal physical address or data error during instruction fetch
PC: 0x3ffe953c
EXCVADDR: 0x3ffe953c

Decoding stack results
0x40206984: loop_task(ETSEvent*) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 193
0x4021e22d: br_i15_montymul at src/int/i15_montmul.c line 287
0x4021deec: br_i15_montymul at src/int/i15_montmul.c line 39
0x40222d29: br_i15_modpow_opt at src/int/i15_modpow2.c line 159
0x4021e504: br_rsa_i15_public at src/rsa/rsa_i15_pub.c line 112
0x401000d0: std::function ::operator()() const at c:\prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\tools\xtensa-lx106-elf\xtensa-lx106-elf\include\c++\4.8.2/functional line 2462
0x401001ac: ets_intr_unlock() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 151
0x401001d0: ets_post(uint8, ETSSignal, ETSParam) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 165
0x401001d0: ets_post(uint8, ETSSignal, ETSParam) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 165
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x4020c8f5: _printf_i at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf_i.c line 194
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x40210b28: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 180
0x4020ca20: _printf_i at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf_i.c line 246
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x4020ca20: _printf_i at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf_i.c line 246
0x40210b28: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 180
0x4020ca20: _printf_i at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf_i.c line 246
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x40100d51: check_poison_neighbors(unsigned short) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\umm_malloc/umm_local.c line 71
0x40210ddc: _svfprintf_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 531
0x401012fc: umm_realloc(void*, size_t) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\umm_malloc\umm_malloc.cpp line 760
0x40100f3d: get_unpoisoned_check_neighbors(void*, char const*, int) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\umm_malloc/umm_local.c line 104
0x40210bf2: __ssputs_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 233
0x4010136d: umm_poison_realloc_fl(void*, size_t, char const*, int) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\umm_malloc/umm_local.c line 140
0x40210ddc: _svfprintf_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/nano-vfprintf.c line 531
0x4020e291: _vsnprintf_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/vsnprintf.c line 73
0x40205cdd: String::changeBuffer(unsigned int) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\WString.cpp line 187
0x40100d51: check_poison_neighbors(unsigned short) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\umm_malloc/umm_local.c line 71
0x40100e19: umm_malloc_core(size_t) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\umm_malloc\umm_malloc.cpp line 422
0x4020e291: _vsnprintf_r at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/vsnprintf.c line 73
0x40100590: free(void*) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\heap.cpp line 259
0x4021243d: operator delete(void*) at /workdir/repo/gcc/libstdc++-v3/libsupc++/del_op.cc line 48
0x40212428: operator delete[](void*) at /workdir/repo/gcc/libstdc++-v3/libsupc++/del_opv.cc line 33
0x4020119c: _GLOBAL__sub_I__ZN16ESP8266WiFiClass9printDiagER5Print() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266WiFi\src/ESP8266WiFi.h line 57
0x40204bf6: Print::printf_P(char const*, ...) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\Print.cpp line 101
0x40100b2c: umm_free_core(void*) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\umm_malloc\umm_malloc.cpp line 316
0x4020e2d4: vsnprintf at /home/earle/src/esp-quick-toolchain/repo/newlib/newlib/libc/stdio/vsnprintf.c line 42
0x402078fd: uart_write(uart_t*, char const*, size_t) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\uart.cpp line 509
0x402045a8: HardwareSerial::write(unsigned char const*, unsigned int) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266/HardwareSerial.h line 165
0x40204bec: Print::printf_P(char const*, ...) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\Print.cpp line 97
0x401001d0: ets_post(uint8, ETSSignal, ETSParam) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 165
0x40206a0b: __yield() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266/core_esp8266_features.h line 92
0x40206a50: optimistic_yield(uint32_t) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 139
0x40202873: WiFiClient::available() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266WiFi\src\WiFiClient.cpp line 263
0x4020a7c4: HTTPClient::connected() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266HTTPClient\src\ESP8266HTTPClient.cpp line 472
0x40208d2c: HTTPClient::disconnect(bool) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266HTTPClient\src\ESP8266HTTPClient.cpp line 435
0x4020415c: EspClass::getFlashChipRealSize() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\Esp.cpp line 293
0x40209aec: HTTPClient::end() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266HTTPClient\src\ESP8266HTTPClient.cpp line 425
0x40203fc5: ESP8266HTTPUpdate::handleUpdate(HTTPClient&, String const&, bool) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266httpUpdate\src\ESP8266httpUpdate.cpp line 444
0x40209b58: HTTPClient::begin(WiFiClient&, String const&) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266HTTPClient\src\ESP8266HTTPClient.cpp line 178
0x40204059: ESP8266HTTPUpdate::update(WiFiClient&, String const&, String const&) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\libraries\ESP8266httpUpdate\src\ESP8266httpUpdate.cpp line 94
0x40205e14: String::copy(char const*, unsigned int) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\WString.cpp line 214
0x40205e4a: String::String(char const*) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\WString.cpp line 36
0x40201131: loop() at C:\Prace\Arduino\ESP8266\httpUpdateSigned/httpUpdateSigned.ino line 101
0x401001d0: ets_post(uint8, ETSSignal, ETSParam) at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 165
0x40206af8: loop_wrapper() at C:\Prg\arduino-1.8.12\hardware\esp8266-git-head\esp8266\cores\esp8266\core_esp8266_main.cpp line 185

[JiriBilek]

There is also a memory leak in

Arduino/cores/esp8266/Updater.cpp

Line 264 in 845241b

if (!_verify->verify(_hash, (void *)sig, sigLen)) {

The sig variable and the buffer needs to be freed. But it doesn't fix the problem..

    if (!_verify->verify(_hash, (void *)sig, sigLen)) {
      _setError(UPDATE_ERROR_SIGN);
      free(sig);
      _reset();
      return false;
    }
    free(sig);

[JiriBilek]

I made a simple sketch to verify the signature on the running sketch (no uploading from the internet etc). The code is taken from

Arduino/cores/esp8266/Updater.cpp

Line 233 in 845241b

_hash->begin();

And it is running fine. So the problem won't be simply in the RSA signature, unfortunately.

verifySig.zip

If you want to play with the code, note that Arduino IDE uploads unsigned sketch. You need to upload the signed one using python script manually.

Edit:
I modified the sketch to open a https client and read data from example.com periodically. So far so good, no problem.

[earlephilhower]

Good catch on the memory leak.

Because the PC has gone bad it feels like a stack corruption issue (as-in stack overflow).

We're now using the standard format signature and not just the SHA256 result, so a change of 1 byte in the signature may end up causing the packed format to go wacky. BearSSL should still not crash things, in that case, though.

Can you give the exact pub/priv key and the exact offset from the end of the .bin.signed you changed, so I can rerun the same changes on my own builds to debug?

[JiriBilek]

The keys are in verifySig.zip in the post above.

Edit: I edited the bin file so that I left the signature intact and changed another byte. Crashing as well.

Edit2: You asked about my changes: I changed offset 0xf0 to value 0 or 0xff to value 0, both crashed.

[JiriBilek]

@earlephilhower:
I added ESP.getFreeContStack(); reporting in the loop of the failing sketch (httpUpdateSecure.ino) and after verifying a bad signature I got this:

[SETUP] WAIT 4...
[SETUP] WAIT 3...
[SETUP] WAIT 2...
[SETUP] WAIT 1...
free cont stack: 3136
HTTP_UPDATE_FAILED Error (12): Update error: ERROR[12]: Signature verification failed
free cont stack: 0

Doesn't it mean stack overflow?

Edit: a similar behaviour I see in my application:
on start: 2888 bytes stack high watermark
after an unsuccessful update (no file present): 1472 bytes
after an unsuccessful update (bad signature): 0 bytes and boom -> wdt

[earlephilhower]

That looks like a smoking gun right there, @JiriBilek. I may need to allocate/use the 2nd stack for the verification, or check that the Updater stack usage is minimized.