Skip to content
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

[TW#12798] components/lwip/apps/dhcpserver: Heap corruption for extended message options #631

Closed
devsaurus opened this issue May 21, 2017 · 5 comments

Comments

@devsaurus
Copy link
Contributor

We've experienced and reported the same issue with NodeMCU based on non-OS SDK in the past. It appears to be still present in the IDF's dhcpserver.

Problem description

When the firmware is in soft-AP mode and DHCP server is enabled, the DHCP server will cause a crash when a packet is received that exceeds the expected size for struct dhcp_msg in dhcpserver.h. This blocks certain clients that send such packets from interfacing with the ESP AP.

Steps to reproduce

Download and extract dhcp_msgs.zip.

  • dhcp.bin contains a valid DHCP request.
  • dhcp_fault.bin adds a lot of 0xdeadbeef in the padding section to extend the size of the DHCP request beyond the size of struct dhcp_msg.

Connect to the ESP AP from a Linux machine. At the shell execute

nc -v -u 192.168.4.1 67 < dhcp_fault.bin

This triggers an immediate reset of the firmware.

Guru Meditation Error of type LoadProhibited occurred on core  1. Exception was unhandled.
Register dump:
PC      : 0x40084deb  PS      : 0x00060133  A0      : 0x80084f48  A1      : 0x3ffbbd10  
A2      : 0x3ffc7c3c  A3      : 0x00000000  A4      : 0xff000000  A5      : 0x80ffffff  
A6      : 0x0000e2a6  A7      : 0x00000090  A8      : 0x00000000  A9      : 0x00000000  
A10     : 0x3ffb1c70  A11     : 0x00000250  A12     : 0x3ffc47e0  A13     : 0x3ffc69c0  
A14     : 0x0204a8c0  A15     : 0x6504a8c0  SAR     : 0x00000010  EXCCAUSE: 0x0000001c  
EXCVADDR: 0x00000000  LBEG    : 0x4000c46c  LEND    : 0x4000c477  LCOUNT  : 0x00000000  

Backtrace: 0x40084deb:0x3ffbbd10 0x40084f48:0x3ffbbd30 0x4008752f:0x3ffbbd50 0x40087570:0x3ffbbd70 0x40082328:0x3ffbbd90 0x4000beb2:0x3ffbbdb0 0x400f67a1:0x3ffbbdd0 0x400f9dfe:0x3ffbbdf0 0x400fa20e:0x3ffbbe30 0x400f7380:0x3ffbbe50 0x400f05d4:0x3ffbbe80 0x400f031d:0x3ffbbea0 0x400f992c:0x3ffbbec0

Rebooting...
@negativekelvin
Copy link
Contributor

negativekelvin commented May 21, 2017

https://github.com/espressif/esp-idf/blob/master/components/lwip/apps/dhcpserver.c#L897
https://github.com/espressif/esp-idf/blob/master/components/lwip/apps/dhcpserver.c#L918

No check for dhcps_msg overflow

This blocks certain clients that send such packets from interfacing with the ESP AP.

Are these packets in spec? How to resolve? Drop the extra bytes?

@devsaurus
Copy link
Contributor Author

Are these packets in spec?

https://www.ietf.org/rfc/rfc2131.txt only states a minimum supported option length for the client - 312 octets. No spec for the server if I didn't miss anything there.

How to resolve? Drop the extra bytes?

I got the recommendation back then to increase the options length from 312 to MTU - IPHEAD(20) - UDPHEAD(8) - DHCPHEAD(236). This accommodates the payload of a full UDP packet. Drawback of this simple solution is that the server starts to reply with these big packets itself since sizeof(struct dhcps_msg) increases accordingly.

@FayeY FayeY changed the title components/lwip/apps/dhcpserver: Heap corruption for extended message options [TW#12798] components/lwip/apps/dhcpserver: Heap corruption for extended message options May 23, 2017
@liuzfesp
Copy link
Contributor

Thanks @devsaurus, your analysis is helpful for us, will update you once we have new progress...

@liuzfesp
Copy link
Contributor

Hi @devsaurus @negativekelvin the fix for this issue is submit, it will be targeted into idf release 2.1
dhcpserver.c.tar.gz

@devsaurus
Copy link
Contributor Author

Thanks for the fix, @liuzfesp!

devsaurus added a commit to devsaurus/nodemcu-firmware that referenced this issue Jun 19, 2017
devsaurus added a commit to devsaurus/nodemcu-firmware that referenced this issue Jun 19, 2017
marcelstoer pushed a commit to nodemcu/nodemcu-firmware that referenced this issue Jun 20, 2017
* backport fix for espressif/esp-idf#631
* remove code from intermediate fix
eiselekd pushed a commit to eiselekd/nodemcu-firmware that referenced this issue Jan 7, 2018
* backport fix for espressif/esp-idf#631
* remove code from intermediate fix
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

No branches or pull requests

3 participants