Created attachment 7881 [details] Corrupt dhcpdiscover message which triggers the out of bounds read Summary ======== An out-of-bounds read can be triggered by functions making calls to dhcp_get_option() in udhcp applet which can potentially result in disclosure of confidential data in memory like stack canary. Affects udhcp client, server and relay. Description ========= It is possible to make udhcp_get_option() return an address just outside the struct by having length as 0 in the last TLV record. This returns a pointer outside the dhcp_packet struct in stack. For some DHCP options which are of constant length (like IPv4 address), the caller does not check if the length is indeed that (4 bytes in this case) but continues to dereference it as a 4 byte value. Also, it is NOT necessary to have a 0 length TLV record to achieve this. One can also do it by specifying a bogus length which is less than what is being expected of length of that particular option. Since get_dhcp_option() is a helper function, it is being consumed by the dhcp client, server and relay at numerous locations. And so, it is potentially vulnerable when any constant sized DHCP option needs to be parsed. Steps to reproduce =============== This is a specific example to trigger the oob read in dhcp server from a malicious client. 1) Build busybox with ASAN enabled. 2) Start the udhcp server with the default minimal example configuration (should not matter which config used) ./busybox_unstripped udhcpd -f ./udhcpd.conf 3) Send the content of the attached corrupted dhcpdiscover message to the DHCP server using any UDP client. In bash, one can do cat corrupt_dhcpdiscover.bin > /dev/udp/127.0.0.1/67 The server will crash as ASAN detects an out of bounds read while trying to parse DHCP_REQUESTED_IP. READ of size 4 at 0xffcc6804 thread T0 #0 0x569f617c in udhcpd_main networking/udhcp/dhcpd.c:1016 Address 0xffcc6804 is located in stack of thread T0 at offset 948 in frame #0 0x569f44fa in udhcpd_main networking/udhcp/dhcpd.c:803 This frame has 5 object(s): [32, 36) 'str_I' [96, 100) 'str_a' [160, 176) 'pfds' [224, 260) 'fake_lease' [320, 948) 'packet' <== Memory access at offset 948 overflows this variable SUMMARY: AddressSanitizer: stack-buffer-overflow networking/udhcp/dhcpd.c:1016 in udhcpd_main Shadow bytes around the buggy address: 0x3ff98cb0: f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98cc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98cd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x3ff98d00:[04]f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98d10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98d20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98d30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98d40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x3ff98d50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -- TRUNCATED ASAN LEGEND -- ==1616==ABORTING
Thanks! Can you build from current git and check whether this commit helped? commit 6d3b4bb24da9a07c263f3c1acf8df85382ff562c Date: Mon Dec 17 18:07:18 2018 +0100 udhcpc: check that 4-byte options are indeed 4-byte, closes 11506 Reopen this BZ if fix is not correct or incomplete
fill_envp() function in dhcpc.c makes calls to udhcp_get_option() in a loop. So, it is not possible to check for the exact length parsed for specific options. So, any options used after fill_envp() parsing may again lead to out of bounds read in client side. Any thoughts about that?
(In reply to Krishna Ram Prakash R from comment #2) > fill_envp() function in dhcpc.c makes calls to udhcp_get_option() in a loop. > So, it is not possible to check for the exact length parsed for specific options. > So, any options used after fill_envp() parsing may again lead to out of bounds read in client side. Any thoughts about that? You need to point out the specific code path where length is not checked. I looked at the code and so far I don't see it.
I was originally talking about the exporting of unknown options which on closer look, I realize it is not an issue and cannot lead to out-of-bounds read as only the required memory is allocated according to the specified length and is null terminated. But, I also noted that in fill_envp(), subnet option is fetched and stored without making sure that the length is of 4 bytes. Shouldn't we also check if the length is 4 if code == DHCP_SUBNET before calling move_from_unaligned()? 531 temp = udhcp_get_option(packet, code); 532 *curr = xmalloc_optname_optval(temp, &dhcp_optflags[i], opt_name); 533 putenv(*curr++); 534 if (code == DHCP_SUBNET) { 535 /* Subnet option: make things like "$ip/$mask" possible */ 536 uint32_t subnet; 537 move_from_unaligned32(subnet, temp); 538 *curr = xasprintf("mask=%u", mton(subnet)); Thanks!
Indeed. I fixed DHCP_SUBNET decoding in git.
Comment on attachment 7881 [details] Corrupt dhcpdiscover message which triggers the out of bounds read 好
(In reply to 金学梅 from comment #6) "><a href="https://chcek">asd</a>