Skip to content

ESP crashes when a UDP packet is sent to port used by ArduinoOTA #3912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mkillewald opened this issue Dec 5, 2017 · 9 comments
Closed

ESP crashes when a UDP packet is sent to port used by ArduinoOTA #3912

mkillewald opened this issue Dec 5, 2017 · 9 comments
Labels
component: libraries type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Milestone

Comments

@mkillewald
Copy link
Contributor

mkillewald commented Dec 5, 2017

Basic Infos

Hardware

Hardware: ESP-12
Core Version: 2.4.0-rc2

Description

ESP will crash and reboot when ArduinoOTA is enabled and a udp packet of any size is sent to the port used by ArduinoOTA. In fact, even just a nmap scan of the port will cause a crash (ex: nmap -sU -p 8266 192.168.1.212)

Settings in IDE

Module: WeMos D1 R2 and mini
Flash Size: 4M (3M SPIFFS)
CPU Frequency: 80Mhz

Sketch

#include <ESP8266WiFi.h>
#include <ESP8266mDNS.h>
#include <WiFiUdp.h>
#include <ArduinoOTA.h>

const char* ssid = ""; // ssid and password erased
const char* password = ""; // ssid and password erased

void setup() {
  Serial.begin(115200);
  Serial.println("Booting");
  WiFi.mode(WIFI_STA);
  WiFi.begin(ssid, password);
  while (WiFi.waitForConnectResult() != WL_CONNECTED) {
    Serial.println("Connection Failed! Rebooting...");
    delay(5000);
    ESP.restart();
  }

  // Port defaults to 8266
  // ArduinoOTA.setPort(8266);

  // Hostname defaults to esp8266-[ChipID]
  // ArduinoOTA.setHostname("myesp8266");

  // No authentication by default
  // ArduinoOTA.setPassword("admin");

  // Password can be set with it's md5 value as well
  // MD5(admin) = 21232f297a57a5a743894a0e4a801fc3
  // ArduinoOTA.setPasswordHash("21232f297a57a5a743894a0e4a801fc3");

  ArduinoOTA.onStart([]() {
    String type;
    if (ArduinoOTA.getCommand() == U_FLASH)
      type = "sketch";
    else // U_SPIFFS
      type = "filesystem";

    // NOTE: if updating SPIFFS this would be the place to unmount SPIFFS using SPIFFS.end()
    Serial.println("Start updating " + type);
  });
  ArduinoOTA.onEnd([]() {
    Serial.println("\nEnd");
  });
  ArduinoOTA.onProgress([](unsigned int progress, unsigned int total) {
    Serial.printf("Progress: %u%%\r", (progress / (total / 100)));
  });
  ArduinoOTA.onError([](ota_error_t error) {
    Serial.printf("Error[%u]: ", error);
    if (error == OTA_AUTH_ERROR) Serial.println("Auth Failed");
    else if (error == OTA_BEGIN_ERROR) Serial.println("Begin Failed");
    else if (error == OTA_CONNECT_ERROR) Serial.println("Connect Failed");
    else if (error == OTA_RECEIVE_ERROR) Serial.println("Receive Failed");
    else if (error == OTA_END_ERROR) Serial.println("End Failed");
  });
  ArduinoOTA.begin();
  Serial.println("Ready");
  Serial.print("IP address: ");
  Serial.println(WiFi.localIP());
}

void loop() {
  ArduinoOTA.handle();
}

Debug Messages

Exception (3):
epc1=0x4010011d epc2=0x00000000 epc3=0x00000000 excvaddr=0x4003bff8 depc=0x00000000

ctx: sys 
sp: 3ffffb70 end: 3fffffb0 offset: 01a0

>>>stack>>>
3ffffd10:  3ffeeea8 00000227 00000227 4010020c  
3ffffd20:  0000a6f0 000014df 3fff1914 401005a6  
3ffffd30:  0004a6e4 00001138 00000227 00000227  
3ffffd40:  00000000 00000002 00000001 3ffffda0  
3ffffd50:  0000a6f0 3ffffdf0 3ffffdf0 402068ef  
3ffffd60:  0000a6e0 00000001 3ffffdf0 4020693b  
3ffffd70:  3ffe8dc3 00000000 3ffffdf0 40206b07  
3ffffd80:  3ffffdb0 3ffeec28 3ffffdf0 00000000  
3ffffd90:  3ffeec28 00000001 3ffffdf0 40206b40  
3ffffda0:  3ffe00ff 00000030 0000000b 40205095  
3ffffdb0:  40104c00 0001c9c2 3ffffdf0 40205101  
3ffffdc0:  0000000a 00000000 401030c5 3ffed4e0  
3ffffdd0:  0000003c 00000000 0000ea60 3ffeb58a  
3ffffde0:  00000000 3ffeec70 3ffeec28 40205218  
3ffffdf0:  3fff1914 0000a6df 0000a6df 40100ec2  
3ffffe00:  00000005 00000000 00000020 40100ec2  
3ffffe10:  3ffe9365 401042bf 3ffecdf0 00000016  
3ffffe20:  40101bdd 3ffecdf0 3ffed4e0 4010340a  
3ffffe30:  00007fff 0235aa7c 3ffed828 40101dae  
3ffffe40:  3ffe9c10 00000000 00000000 00000002  
3ffffe50:  00007fff 0235aa7c 401021ee 00000100  
3ffffe60:  7fffffff 3ffe9c10 3ffe9c10 00000001  
3ffffe70:  00000001 00004a88 00002200 4000050c  
3ffffe80:  00000000 0235aa7c 00002200 4000050c  
3ffffe90:  3fffc278 40101f88 3fffc200 00000022  
3ffffea0:  3ffe0000 00000030 0000001b ffffffff  
3ffffeb0:  3fff0c8c 3fff168c 3fff1214 40207387  
3ffffec0:  40000f3d 00000023 00000000 40203588  
3ffffed0:  3fff0c8c 3fff168c 3fff13fc 4022837c  
3ffffee0:  0000ae6b 00000000 3ffeb576 3fff0674  
3ffffef0:  000000c0 3fffdad0 3ffeef90 401067ac  
3fffff00:  40220000 00000000 3ffed270 3fff0670  
3fffff10:  3ffeb576 3fff0674 3fff168c 40229a8c  
3fffff20:  3fff04c4 3fff0c8c 3fff0c8c 3ffedf78  
3fffff30:  00000000 3fff168c 0000001c 3fff0c8c  
3fffff40:  3ffeb568 00000000 3fff168c 40228e9d  
3fffff50:  f701a8c0 00000052 00000000 00000020  
3fffff60:  00000002 0000001a 4021ede7 3ffecdf0  
3fffff70:  3ffeb540 3fffdcc0 3ffe9478 3ffe9478  
3fffff80:  4021ed5a 3ffecdf0 00000000 3fff0cd4  
3fffff90:  3fffdc80 00000000 3fff168c 4022cacf  
3fffffa0:  40000f49 3fffdab0 3fffdab0 40000f49  
<<<stack<<<

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 1384, room 16 
tail 8
chksum 0x2d
csum 0x2d
v0c897c37
~ld
Booting
Ready
IP address: 192.168.1.212

Decoding 34 results
0x4010020c: _umm_free at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/umm_malloc/umm_malloc.c line 1291
0x401005a6: _umm_realloc at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/umm_malloc/umm_malloc.c line 1572
:  (inlined by) realloc at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/umm_malloc/umm_malloc.c line 1713
0x4010340a: lmacProcessTxSuccess at ?? line ?
0x402068ef: String::changeBuffer(unsigned int) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/WString.cpp line 518
0x4020693b: String::reserve(unsigned int) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/WString.cpp line 518
0x40206b07: String::concat(char const*, unsigned int) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/WString.cpp line 518
0x40206b40: String::concat(char) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/WString.cpp line 518
0x40205095: ArduinoOTAClass::parseInt() at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/libraries/ESP8266WiFi/src/include/UdpContext.h line 399
0x40104c00: ets_timer_arm_new at ?? line ?
0x40205101: String::operator+=(char) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/libraries/ESP8266WiFi/src/include/UdpContext.h line 399
:  (inlined by) ArduinoOTAClass::readStringUntil(char) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/libraries/ArduinoOTA/ArduinoOTA.cpp line 168
0x40205218: ArduinoOTAClass::_onRx() at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/libraries/ArduinoOTA/ArduinoOTA.cpp line 187
0x401034c3: lmacRecycleMPDU at ?? line ?
0x40103926: lmacRecycleMPDU at ?? line ?
0x402173de: cnx_update_bss_more at ?? line ?
0x4010340a: lmacProcessTxSuccess at ?? line ?
0x40214fd5: scan_parse_beacon at ?? line ?
0x4010228b: wDev_ProcessFiq at ?? line ?
0x40101f88: wDev_ProcessFiq at ?? line ?
0x40104c70: ets_timer_arm_new at ?? line ?
0x401006eb: cont_run at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/cont.S line 79
0x40206dfb: loop_task at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/core_esp8266_main.cpp line 57
0x40206e6c: loop_wrapper at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/core_esp8266_main.cpp line 57
0x40207387: std::_Function_handler    (ArduinoOTAClass*)> >::_M_invoke(std::_Any_data const&) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/1.20.0-26-gb404fb9-2/xtensa-lx106-elf/include/c++/4.8.2/functional line 2073
0x40203588: UdpContext::_s_recv(void*, udp_pcb*, pbuf*, ip_addr*, unsigned short) at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/libraries/ESP8266mDNS/ESP8266mDNS.cpp line 394
0x4022837c: udp_input at core/udp.c line 343
0x401067ac: pvPortMalloc at /Users/k1ds3ns4t10n/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.0-rc2/cores/esp8266/heap.c line 40
0x40229a8c: ip_input at core/ipv4/ip.c line 553
0x40228e9d: ethernet_input at netif/etharp.c line 1379
0x4021ede7: pp_tx_idle_timeout at ?? line ?
0x4021ed5a: pp_tx_idle_timeout at ?? line ?
0x4022cacf: ets_snprintf at ?? line ?
@yoursunny
Copy link
Contributor

Although I do not know how ArduinoOTA is supposed to work, I can reproduce this issue by sending a short packet using echo -n ABCDEFGH > /dev/udp/192.168.137.229/8266.

@yoursunny
Copy link
Contributor

Another packet to trigger a crash is:

echo -n 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890 > /dev/udp/192.168.137.229/8266

The stacktrace appears to be:

0x402038b4: UdpContext::_s_recv(void*, udp_pcb*, pbuf*, ip4_addr const*, unsigned short) at D:\home\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266WiFi\src/WiFiUdp.cpp line 282
0x40203904: UdpContext::_s_recv(void*, udp_pcb*, pbuf*, ip4_addr const*, unsigned short) at D:\home\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266WiFi\src/WiFiUdp.cpp line 282
0x402115d4: udp_input at /home/david/dev/esp8266/origin/tools/sdk/lwip2/builder/lwip2-src/src/core/udp.c line 463
0x40214aa8: ip4_input at /home/david/dev/esp8266/origin/tools/sdk/lwip2/builder/lwip2-src/src/core/ipv4/ip4.c line 679
0x4020e2c9: ethernet_input_LWIP2 at /home/david/dev/esp8266/origin/tools/sdk/lwip2/builder/lwip2-src/src/netif/ethernet.c line 182
0x401048e4: esp2glue_ethernet_input at /home/david/dev/esp8266/origin/tools/sdk/lwip2/builder/glue-lwip/lwip-git.c line 406
0x4022b39e: ethernet_input at /home/david/dev/esp8266/origin/tools/sdk/lwip2/builder/glue-esp/lwip-esp.c line 349
0x4022b3cc: ethernet_input at /home/david/dev/esp8266/origin/tools/sdk/lwip2/builder/glue-esp/lwip-esp.c line 357
0x4022631b: ppPeocessRxPktHdr at ?? line ?
0x4022d41b: ets_snprintf at ?? line ?

I say "appears to be" because the logic in ArduinoOTAClass:parseInt has a buffer overflow vulnerability and is overwriting part of the stack.

yoursunny added a commit to yoursunny/ESP8266-Arduino that referenced this issue Jan 4, 2018
yoursunny added a commit to yoursunny/ESP8266-Arduino that referenced this issue Jan 4, 2018
@yoursunny
Copy link
Contributor

#4086 has fixes for both bad packets.

@mkillewald
Copy link
Contributor Author

Awesome. I will try it as soon as I can. Thank you!

@devyte devyte closed this as completed in d9ef6b5 Jan 4, 2018
@devyte
Copy link
Collaborator

devyte commented Jan 4, 2018

Already merged after review. Please report if it fixes your issue.

@devyte devyte added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jan 4, 2018
@mkillewald
Copy link
Contributor Author

Ok so good news and bad news. While the changes @yoursunny made did stop the ESP from crashing, it seems that they also broke OTA as I get [ERROR]: No Answer when trying to upload via OTA using the example sketch.

I upgraded from 2.4.0-rc2 to 2.4.0 before applying the changes to ArduinoOTA.cpp. If I roll back the changes to ArduinoOTA.cpp, uploads via OTA again work but the crash condition also still occurs.

@mkillewald
Copy link
Contributor Author

Had a bit more time to play around with it this morning. If I keep the changes @yoursunny made to readStringUntil() but roll back the changes made to parseInt() everything is good. No more crash and OTA updates are working with the example sketch. Sorry, I'm kinda new to all of this. Should I send a new pull request, or should @yoursunny resubmit?

@yoursunny
Copy link
Contributor

If you rollback parseInt there will be crash upon receiving a packet with a long string of digits.

echo -n 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890 > /dev/udp/192.168.137.229/8266

Please try changing both index++ to index. If it works, you may submit PR.

@mkillewald
Copy link
Contributor Author

That did it! No crash and OTA updates are working. PR #4092 submitted.

incosystem pushed a commit to incosys/Arduino that referenced this issue Jan 8, 2018
* ArduinoOTA: handle end of packet in readStringUntil

fixes esp8266#3912

* ArduinoOTA: fix buffer overflow in parseInt

fixes esp8266#3912
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: libraries type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

No branches or pull requests

4 participants