Skip to content

Commit ab46684

Browse files
0xc01700xc0170
authored
Merge pull request #3336 from mikaleppanen/lwip_dhcp_parse_corr
Squashed 'features/FEATURE_LWIP/lwip-interface/lwip/' changes from d7…
2 parents b08e1ec + 420d5c3 commit ab46684

File tree

3 files changed

+114
-2
lines changed

3 files changed

+114
-2
lines changed

‎features/FEATURE_LWIP/lwip-interface/lwip/CHANGELOG

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,9 @@ HISTORY
328328

329329
++ Bugfixes:
330330

331+
2016-11-25: Simon Goldschmidt
332+
* dhcp.c: fixed bug #49676 (Possible endless loop when parsing dhcp options)
333+
331334
2016-07-20: Simon Goldschmidt
332335
* memp.h/.c: fixed bug #48442 (memp stats don't work for MEMP_MEM_MALLOC)
333336

‎features/FEATURE_LWIP/lwip-interface/lwip/src/core/ipv4/lwip_dhcp.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,6 +1535,8 @@ dhcp_parse_reply(struct dhcp *dhcp, struct pbuf *p)
15351535
#endif /* LWIP_DHCP_GET_NTP_SRV*/
15361536
case(DHCP_OPTION_OVERLOAD):
15371537
LWIP_ERROR("len == 1", len == 1, return ERR_VAL;);
1538+
/* decode overload only in options, not in file/sname: invalid packet */
1539+
LWIP_ERROR("overload in file/sname", options_idx == DHCP_OPTIONS_OFS, return ERR_VAL;);
15381540
decode_idx = DHCP_OPTION_IDX_OVERLOAD;
15391541
break;
15401542
case(DHCP_OPTION_MESSAGE_TYPE):

‎features/FEATURE_LWIP/lwip-interface/lwip/test/unit/dhcp/test_dhcp.c

Lines changed: 109 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ static enum tcase {
118118
TEST_LWIP_DHCP,
119119
TEST_LWIP_DHCP_NAK,
120120
TEST_LWIP_DHCP_RELAY,
121-
TEST_LWIP_DHCP_NAK_NO_ENDMARKER
121+
TEST_LWIP_DHCP_NAK_NO_ENDMARKER,
122+
TEST_LWIP_DHCP_INVALID_OVERLOAD
122123
} tcase;
123124

124125
static int debug = 0;
@@ -900,6 +901,111 @@ START_TEST(test_dhcp_nak_no_endmarker)
900901
}
901902
END_TEST
902903

904+
START_TEST(test_dhcp_invalid_overload)
905+
{
906+
u8_t dhcp_offer_invalid_overload[] = {
907+
0x00, 0x23, 0xc1, 0xde, 0xd0, 0x0d, /* To unit */
908+
0x00, 0x0F, 0xEE, 0x30, 0xAB, 0x22, /* From Remote host */
909+
0x08, 0x00, /* Protocol: IP */
910+
0x45, 0x10, 0x01, 0x48, 0x00, 0x00, 0x00, 0x00, 0x80, 0x11, 0x36, 0xcc, 0xc3, 0xaa, 0xbd, 0xab, 0xc3, 0xaa, 0xbd, 0xc8, /* IP header */
911+
0x00, 0x43, 0x00, 0x44, 0x01, 0x34, 0x00, 0x00, /* UDP header */
912+
913+
0x02, /* Type == Boot reply */
914+
0x01, 0x06, /* Hw Ethernet, 6 bytes addrlen */
915+
0x00, /* 0 hops */
916+
0xAA, 0xAA, 0xAA, 0xAA, /* Transaction id, will be overwritten */
917+
0x00, 0x00, /* 0 seconds elapsed */
918+
0x00, 0x00, /* Flags (unicast) */
919+
0x00, 0x00, 0x00, 0x00, /* Client ip */
920+
0xc3, 0xaa, 0xbd, 0xc8, /* Your IP */
921+
0xc3, 0xaa, 0xbd, 0xab, /* DHCP server ip */
922+
0x00, 0x00, 0x00, 0x00, /* relay agent */
923+
0x00, 0x23, 0xc1, 0xde, 0xd0, 0x0d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* MAC addr + padding */
924+
925+
/* Empty server name */
926+
0x34, 0x01, 0x02, 0xff, /* Overload: SNAME + END */
927+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
928+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
929+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
930+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
931+
/* Empty boot file name */
932+
0x34, 0x01, 0x01, 0xff, /* Overload FILE + END */
933+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
934+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
935+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
936+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
937+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
938+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
939+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
940+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
941+
942+
0x63, 0x82, 0x53, 0x63, /* Magic cookie */
943+
0x35, 0x01, 0x02, /* Message type: Offer */
944+
0x36, 0x04, 0xc3, 0xaa, 0xbd, 0xab, /* Server identifier (IP) */
945+
0x33, 0x04, 0x00, 0x00, 0x00, 0x78, /* Lease time 2 minutes */
946+
0x03, 0x04, 0xc3, 0xaa, 0xbd, 0xab, /* Router IP */
947+
0x01, 0x04, 0xff, 0xff, 0xff, 0x00, /* Subnet mask */
948+
0x34, 0x01, 0x03, /* Overload: FILE + SNAME */
949+
0xff, /* End option */
950+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
951+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* Padding */
952+
};
953+
ip4_addr_t addr;
954+
ip4_addr_t netmask;
955+
ip4_addr_t gw;
956+
u32_t xid;
957+
LWIP_UNUSED_ARG(_i);
958+
959+
tcase = TEST_LWIP_DHCP_INVALID_OVERLOAD;
960+
setdebug(0);
961+
962+
IP4_ADDR(&addr, 0, 0, 0, 0);
963+
IP4_ADDR(&netmask, 0, 0, 0, 0);
964+
IP4_ADDR(&gw, 0, 0, 0, 0);
965+
966+
netif_add(&net_test, &addr, &netmask, &gw, &net_test, testif_init, ethernet_input);
967+
netif_set_up(&net_test);
968+
969+
dhcp_start(&net_test);
970+
971+
fail_unless(txpacket == 1); /* DHCP discover sent */
972+
xid = htonl(netif_dhcp_data(&net_test)->xid);
973+
memcpy(&dhcp_offer_invalid_overload[46], &xid, 4); /* insert correct transaction id */
974+
dhcp_offer_invalid_overload[311] = 3;
975+
send_pkt(&net_test, dhcp_offer_invalid_overload, sizeof(dhcp_offer_invalid_overload));
976+
/* IP addresses should be zero */
977+
fail_if(memcmp(&addr, &net_test.ip_addr, sizeof(ip4_addr_t)));
978+
fail_if(memcmp(&netmask, &net_test.netmask, sizeof(ip4_addr_t)));
979+
fail_if(memcmp(&gw, &net_test.gw, sizeof(ip4_addr_t)));
980+
fail_unless(txpacket == 1); /* Nothing more sent */
981+
982+
dhcp_offer_invalid_overload[311] = 2;
983+
send_pkt(&net_test, dhcp_offer_invalid_overload, sizeof(dhcp_offer_invalid_overload));
984+
/* IP addresses should be zero */
985+
fail_if(memcmp(&addr, &net_test.ip_addr, sizeof(ip4_addr_t)));
986+
fail_if(memcmp(&netmask, &net_test.netmask, sizeof(ip4_addr_t)));
987+
fail_if(memcmp(&gw, &net_test.gw, sizeof(ip4_addr_t)));
988+
fail_unless(txpacket == 1); /* Nothing more sent */
989+
990+
dhcp_offer_invalid_overload[311] = 1;
991+
send_pkt(&net_test, dhcp_offer_invalid_overload, sizeof(dhcp_offer_invalid_overload));
992+
/* IP addresses should be zero */
993+
fail_if(memcmp(&addr, &net_test.ip_addr, sizeof(ip4_addr_t)));
994+
fail_if(memcmp(&netmask, &net_test.netmask, sizeof(ip4_addr_t)));
995+
fail_if(memcmp(&gw, &net_test.gw, sizeof(ip4_addr_t)));
996+
fail_unless(txpacket == 1); /* Nothing more sent */
997+
998+
dhcp_offer_invalid_overload[311] = 0;
999+
send_pkt(&net_test, dhcp_offer_invalid_overload, sizeof(dhcp_offer));
1000+
1001+
fail_unless(netif_dhcp_data(&net_test)->state == DHCP_STATE_REQUESTING);
1002+
1003+
fail_unless(txpacket == 2); /* No more sent */
1004+
xid = htonl(netif_dhcp_data(&net_test)->xid); /* xid updated */
1005+
1006+
netif_remove(&net_test);
1007+
}
1008+
END_TEST
9031009

9041010
/** Create the suite including all tests for this module */
9051011
Suite *
@@ -909,7 +1015,8 @@ dhcp_suite(void)
9091015
TESTFUNC(test_dhcp),
9101016
TESTFUNC(test_dhcp_nak),
9111017
TESTFUNC(test_dhcp_relayed),
912-
TESTFUNC(test_dhcp_nak_no_endmarker)
1018+
TESTFUNC(test_dhcp_nak_no_endmarker),
1019+
TESTFUNC(test_dhcp_invalid_overload)
9131020
};
9141021
return create_suite("DHCP", tests, sizeof(tests)/sizeof(testfunc), dhcp_setup, dhcp_teardown);
9151022
}

0 commit comments

Comments
 (0)