esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
16.08k stars 13.33k forks source link

WifiClientSecure produce crash when used with client.available() and client.read(). #813

Closed martinayotte closed 9 years ago

martinayotte commented 9 years ago

The following code is working fine, although it seems that there is a delay between header and body :

while (client.connected()) {
  String line = client.readStringUntil('\r');
  p.println(line);
}

But if it is replaced by the following code, it produce a crash :

while (client.connected()) {
  while ((size = client.available()) > 0) {
    uint8_t* msg = (uint8_t*)malloc(size);
    size = client.read(msg, size);
    p.write(msg, size);
    free(msg);
  }
}

The stack trace look like :

Exception (9): epc1=0x4010177f epc2=0x00000000 epc3=0x00000000 excvaddr=0x3fff596b depc=0x00000000

ctx: cont sp: 3ffecea0 end: 3ffed4c0 offset: 01a0

stack>>> 3ffed040: 4020e1e5 00000001 3fff57c8 4020eb96
3ffed050: 00000000 3ffeb804 00000001 40203db9
3ffed060: 3fff57c8 00000005 00000005 00000000
3ffed070: 00000000 3fff5a10 00000000 00000000
3ffed080: 00000000 00000000 00000000 00000000
3ffed090: 00000000 00000000 00000000 00000000
3ffed0a0: 00000000 00000000 00000000 00000000
3ffed0b0: 00000000 00000000 00000000 00000000
3ffed0c0: 00000000 00000000 00000000 00000000
3ffed0d0: 00000000 00000000 00000000 00000000
3ffed0e0: 00000000 00000000 00000000 00000000
3ffed0f0: 00000000 00000000 00000000 00000000
3ffed100: 00000000 00000000 00000000 00000000
3ffed110: 00000000 00000000 00000000 00000000
3ffed120: 00000000 00000000 000 0000 00000000
3ffed130: 00000000 00000000 00000000 00000000
3ffed140: 00000000 00000000 00000000 00000000
3ffed150: 00000000 00000000 00000000 00000000
3ffed160: 00000000 00000000 00000000 00000000
3ffed170: 00000000 00000000 00000000 00000000
3ffed180: 00000000 00000000 00000000 00000000
3ffed190: 00000000 00000000 00000000 00000000
3ffed1a0: 00000000 00000000 00000000 00000000
3ffed1b0: 00000000 00000000 00000000 00000000
3ffed1c0: 00000000 00000000 0000001f 401015c9
3ffed1d0: 4000050c 00000000 00000000 00000000
3ffed1e0: 40002eee 00000030 0000001f ffffffff
3ffed1f0: 40002ef1 05e03079 00000013 00000490
3ffed200: 00000007 00000762 00400762 60000a00
3ffed210: 01280062 00000076 ffffff83 00000640
3ffed220: 05e02be9 00000064 00000063 00000030
3ffed230: 00000000 00000000 00000000 00000000
3ffed240: 00000000 00000000 00000000 00000000
3ffed250: 00000000 00000000 00000000 00000000
3ffed260: 00000000 00000000 00000000 00000000
3ffed270: 00000000 00000000 00000000 00000000
3ffed280: 00000000 00000000 00000000 00000000
3ffed290: 00000008 00000000 00000020 40103faf
3ffed2a0: 40103c6d 00000002 00000000 00000000
3ffed2b0: 3ffedfe0 40103b56 00000001 00000000
3ffed2c0: 00000000 40103927 00000000 3ffedfb0
3ffed2d0: 00000005 00000000 00000020 40103faf
3ffed2e0: 4010383b 3fff1ea0 3ffedfb0 40103faf
3ffed2f0: 40104b99 3fff1ea0 3ffedfe0 40103faf
3ffed300: 00000065 037bc951 3fff32d8 40104d28
3ffed310: 3ffeec6c 00000000 00000000 3ffeec6c
3ffed320: 4010293e 00040000 00000000 7fffffff
3ffed330: 00000022 4010293b 00040000 00000001
3ffed340: 3ffedfe0 4010509a 40105142 00000100
3ffed350: 00000000 037bc951 00000000 3ffeec84
3ffed360: 4000050c 3fffc278 40104ef4 3fffc200
3ffed370: 00000022 4010293b 00040000 3ffeec60
3ffed380: 40000f83 00000030 0000001e ffffffff
3ffed390: 40000ea3 00000023 00000001 00200000
3ffed3a0: 3fffdab0 00000000 3fffdb20 3ffed4ec
3ffed3b0: 00000000 00000000 00000001 3ffed4e4
3ffed3c0: 00000000 3fffdc20 3ffed4ec 00000030
3ffed3d0: 40201c15 3ffec4a0 40201ccc 00000000
3ffed3e0: 00000000 00000000 00000000 ffdfffff
3ffed3f0: ffffffff 3fffc6fc 00000001 3ffec4a0
3ffed400: 00000000 3fffdc20 3ffed4ec 00000030
3ffed410: 3ffec46c 00000001 3ffe9fd4 4020f178
3ffed420: 3ffec46c 00000012 3ffe9ea1 3ffed4ec
3ffed430: 3ffec46c 00000013 3ffec46c 4020a4f9
3ffed440: 3ffe9fd3 3ffec1b8 3ffec46c 4020a4f9
3ffed450: 3ffe9e8f 40207226 3ffec46c 4020f24c
3ffed460: 3ffec46c 3fff5977 00000005 00000005
3ffed470: 3fff41a0 3ffec1b8 3ffeb834 402032e8
3ffed480: 00000000 00000000 00000016 40101dc1
3ffed490: 40201c8d 0104a8c0 00000000 3ffed4ec
3ffed4a0: 3fffdc20 00000000 3ffed4e4 40201cea
3ffed4b0: 00000000 00000000 3ffec4a0 40100398
<<<stack<<<

ets Jan 8 2013,rst cause:1, boot mode:(3,7)

load 0x4010f000, len 1264, room 16 tail 0 chksum 0x42 csum 0x42 ~ld

Links2004 commented 9 years ago

Exception (9) means an unaligned access, can you provide a objdump for the address around 0x4010177f? $(XTENSA_TOOLS_ROOT)/xtensa-lx106-elf-objdump" -S $(PNAME).elf

martinayotte commented 9 years ago

Hi Markus/Links2004 ,

Here it is :

40101768 -vPortFree-:
40101768: f0c112 addi a1, a1, -16 4010176b: 11c9 s32i.n a12, a1, 4 4010176d: 0109 s32i.n a0, a1, 0 4010176f: 02cd mov.n a12, a2
40101771: f29c beqz.n a2, 40101794 -vPortFree+0x2c- 40101773: ff7901 l32r a0, 40101558 -ets_timer_setfn+0x78- 40101776: 0000c0 callx0 a0 40101779: ffcf31 l32r a3, 401016b8 -xPortWantedSizeAlign+0x14- 4010177c: ffdc02 addmi a0, a12, 0xffffff00 4010177f: 3d2002 l32i a0, a0, 244 40101782: 4348 l32i.n a4, a3, 16 40101784: f0cc22 addi a2, a12, -16 40101787: 004a add.n a0, a0, a4 40101789: 4309 s32i.n a0, a3, 16 4010178b: 000905 call0 4010181c -pvPortRealloc+0x38- 4010178e: ff7301 l32r a0, 4010155c -ets_timer_setfn+0x7c- 40101791: 0000c0 callx0 a0 40101794: 11c8 l32i.n a12, a1, 4 40101796: 0108 l32i.n a0, a1, 0 40101798: 10c112 addi a1, a1, 16 4010179b: f00d ret.n 4010179d: 000000 ill 401017a0: 0018a4 excw 401017a3: 223040 excw

Is it in Espressif code ? I'm elligible to BugBounty ? :-)

BTW, why I had to change all brackets by "-" to make it readable in this post, markups seems to different from other sites, backticks doesn't work properly for inline code blocks ?

Links2004 commented 9 years ago

what is the name of the function? the address points in to the code RAM. there can be our code or Espressif code. normally there is a line like: 401017a0 <ets_timer_disarm>: which name the function.

martinayotte commented 9 years ago

It is mentioned above : <vPortFree>

Links2004 commented 9 years ago

vPortFree is from the SDK. check the code if you try to use free to an null ptr

igrr commented 9 years ago

Highly likely that this is the same heap corruption issue as this one: https://github.com/esp8266/Arduino/issues/428

I have been working on that issue for three days (Saturday, Sunday, Monday), and made some progress (found an issue with String class reallocation). However, I haven't yet figured it out. There is some code which does out-of-bounds writes, and eventually this leads to heap corruption. It's been hard to trace because we only have one data breakpoint.

On Thu, Sep 24, 2015 at 6:21 PM, Markus notifications@github.com wrote:

vPortFree is from the SDK. check the code if you try to use free to an null ptr

— Reply to this email directly or view it on GitHub https://github.com/esp8266/Arduino/issues/813#issuecomment-142961018.

martinayotte commented 9 years ago

Ok ! I will add validation in the code above. But this means that the malloc() failed for some reason, I will need to investigate that too, because, as you can see above, the code is quite simple.

igrr commented 9 years ago

You can see this isn't a null ptr access:

Exception (9):
epc1=0x4010177f epc2=0x00000000 epc3=0x00000000 excvaddr=0x3fff596b depc=0x00000000

because excvaddr isn't close to 0.

martinayotte commented 9 years ago

Hi IGRR, If the case you've mentioned is hard to reproduce, maybe you can rely on my use-case, since the corruption seems to be almost instantaneous.

igrr commented 9 years ago

On the contrary, It's quite easy to reproduce it. There's a bunch of examples in the issue I've linked above.

martinayotte commented 9 years ago

Ah ! So, let hope that the good angels are with you :-)

igrr commented 9 years ago

With the change in 72c9033 your sample seems to work fine. I have replaced variable p with Serial because I didn't know what it was. Here's the full sketch i used: https://gist.github.com/igrr/ae22f35a6d151f5fb622

martinayotte commented 9 years ago

Hi Ivan, Thanks for the fix ! So, angels were in GCC-vs-XCC ... :-0 (Yes, the p was for Print class since I use both Serial and/or WiFiClient with the same function)