diff --git a/src/core/ipv4/icmp.c b/src/core/ipv4/icmp.c index cee48c53..9810b9bb 100644 --- a/src/core/ipv4/icmp.c +++ b/src/core/ipv4/icmp.c @@ -62,7 +62,7 @@ #define LWIP_ICMP_ECHO_CHECK_INPUT_PBUF_LEN 1 #endif /* LWIP_ICMP_ECHO_CHECK_INPUT_PBUF_LEN */ -/* The amount of data from the original packet to return in a dest-unreachable */ +/* The maximum amount of data from the original packet to return in a dest-unreachable */ #define ICMP_DEST_UNREACH_DATASIZE 8 static void icmp_send_response(struct pbuf *p, u8_t type, u8_t code); @@ -344,23 +344,27 @@ icmp_send_response(struct pbuf *p, u8_t type, u8_t code) struct icmp_hdr *icmphdr; ip4_addr_t iphdr_src; struct netif *netif; + u16_t response_pkt_len; /* increase number of messages attempted to send */ MIB2_STATS_INC(mib2.icmpoutmsgs); - /* ICMP header + IP header + 8 bytes of data */ - q = pbuf_alloc(PBUF_IP, sizeof(struct icmp_hdr) + IP_HLEN + ICMP_DEST_UNREACH_DATASIZE, - PBUF_RAM); + /* Keep IP header + up to 8 bytes */ + response_pkt_len = IP_HLEN + ICMP_DEST_UNREACH_DATASIZE; + if (p->tot_len < response_pkt_len) response_pkt_len = p->tot_len; + + /* ICMP header + part of original packet */ + q = pbuf_alloc(PBUF_IP, sizeof(struct icmp_hdr) + response_pkt_len, PBUF_RAM); if (q == NULL) { - LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded: failed to allocate pbuf for ICMP packet.\n")); + LWIP_DEBUGF(ICMP_DEBUG, ("icmp_send_response: failed to allocate pbuf for ICMP packet.\n")); MIB2_STATS_INC(mib2.icmpouterrors); return; } LWIP_ASSERT("check that first pbuf can hold icmp message", - (q->len >= (sizeof(struct icmp_echo_hdr) + IP_HLEN + ICMP_DEST_UNREACH_DATASIZE))); + (q->len >= (sizeof(struct icmp_hdr) + response_pkt_len))); iphdr = (struct ip_hdr *)p->payload; - LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded from ")); + LWIP_DEBUGF(ICMP_DEBUG, ("icmp_send_response: Sending ICMP type %02X for packet from ", type)); ip4_addr_debug_print_val(ICMP_DEBUG, iphdr->src); LWIP_DEBUGF(ICMP_DEBUG, (" to ")); ip4_addr_debug_print_val(ICMP_DEBUG, iphdr->dest); @@ -372,8 +376,7 @@ icmp_send_response(struct pbuf *p, u8_t type, u8_t code) icmphdr->data = 0; /* copy fields from original packet */ - SMEMCPY((u8_t *)q->payload + sizeof(struct icmp_echo_hdr), (u8_t *)p->payload, - IP_HLEN + ICMP_DEST_UNREACH_DATASIZE); + pbuf_copy_partial_pbuf(q, p, response_pkt_len, sizeof(struct icmp_hdr)); ip4_addr_copy(iphdr_src, iphdr->src); #ifdef LWIP_HOOK_IP4_ROUTE_SRC diff --git a/test/unit/ip4/test_ip4.c b/test/unit/ip4/test_ip4.c index 4248bf95..3fe484f6 100644 --- a/test/unit/ip4/test_ip4.c +++ b/test/unit/ip4/test_ip4.c @@ -1,5 +1,6 @@ #include "test_ip4.h" +#include "lwip/icmp.h" #include "lwip/ip4.h" #include "lwip/etharp.h" #include "lwip/inet_chksum.h" @@ -17,6 +18,8 @@ static struct netif test_netif; static ip4_addr_t test_ipaddr, test_netmask, test_gw; static int linkoutput_ctr; static int linkoutput_byte_ctr; +static int linkoutput_pkt_len; +static u8_t linkoutput_pkt[100]; /* reference internal lwip variable in netif.c */ @@ -27,6 +30,8 @@ test_netif_linkoutput(struct netif *netif, struct pbuf *p) fail_unless(p != NULL); linkoutput_ctr++; linkoutput_byte_ctr += p->tot_len; + /* Copy start of packet into buffer */ + linkoutput_pkt_len = pbuf_copy_partial(p, linkoutput_pkt, sizeof(linkoutput_pkt), 0); return ERR_OK; } @@ -264,6 +269,54 @@ START_TEST(test_ip4addr_aton) } END_TEST +/* Test for bug #59364 */ +START_TEST(test_ip4_icmp_replylen_short) +{ + /* IP packet to 192.168.0.1 using proto 0x22 and 1 byte payload */ + const u8_t unknown_proto[] = { + 0x45, 0x00, 0x00, 0x15, 0xd4, 0x31, 0x00, 0x00, 0xff, 0x22, + 0x66, 0x41, 0xc0, 0xa8, 0x00, 0x02, 0xc0, 0xa8, 0x00, 0x01, + 0xaa }; + struct pbuf *p; + const int icmp_len = IP_HLEN + sizeof(struct icmp_hdr); + + test_netif_add(); + test_netif.output = arpless_output; + p = pbuf_alloc(PBUF_IP, sizeof(unknown_proto), PBUF_RAM); + pbuf_take(p, unknown_proto, sizeof(unknown_proto)); + fail_unless(ip4_input(p, &test_netif) == ERR_OK); + + fail_unless(linkoutput_ctr == 1); + /* Verify outgoing ICMP packet has no extra data */ + fail_unless(linkoutput_byte_ctr == icmp_len + sizeof(unknown_proto)); + fail_if(memcmp(&linkoutput_pkt[icmp_len], unknown_proto, sizeof(unknown_proto))); +} +END_TEST + +START_TEST(test_ip4_icmp_replylen_first_8) +{ + /* IP packet to 192.168.0.1 using proto 0x22 and 11 bytes payload */ + const u8_t unknown_proto[] = { + 0x45, 0x00, 0x00, 0x1f, 0xd4, 0x31, 0x00, 0x00, 0xff, 0x22, + 0x66, 0x37, 0xc0, 0xa8, 0x00, 0x02, 0xc0, 0xa8, 0x00, 0x01, + 0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8, 0xa9, + 0xaa }; + struct pbuf *p; + const int icmp_len = IP_HLEN + sizeof(struct icmp_hdr); + const int unreach_len = IP_HLEN + 8; + + test_netif_add(); + test_netif.output = arpless_output; + p = pbuf_alloc(PBUF_IP, sizeof(unknown_proto), PBUF_RAM); + pbuf_take(p, unknown_proto, sizeof(unknown_proto)); + fail_unless(ip4_input(p, &test_netif) == ERR_OK); + + fail_unless(linkoutput_ctr == 1); + fail_unless(linkoutput_byte_ctr == icmp_len + unreach_len); + fail_if(memcmp(&linkoutput_pkt[icmp_len], unknown_proto, IP_HLEN + 8)); +} +END_TEST + /** Create the suite including all tests for this module */ Suite * ip4_suite(void) @@ -273,6 +326,8 @@ ip4_suite(void) TESTFUNC(test_ip4_reass), TESTFUNC(test_127_0_0_1), TESTFUNC(test_ip4addr_aton), + TESTFUNC(test_ip4_icmp_replylen_short), + TESTFUNC(test_ip4_icmp_replylen_first_8), }; return create_suite("IPv4", tests, sizeof(tests)/sizeof(testfunc), ip4_setup, ip4_teardown); }