mirror/lwip
1
0
Fork 0
mirror of https://github.com/lwip-tcpip/lwip.git synced 2025-01-26 09:35:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

[PATCH] Drop instead of ASSERT in tcp_input header parsing

Browse Source 
Since allowing input validation to trip the ASSERT handler is bad,
let's just drop the packets instead if validation fails.

Signed-off-by: sg <goldsimon@gmx.de>
This commit is contained in:
Henrik Persson 2016-06-13 20:08:43 +02:00 committed by sg
parent 25652254a5
commit b9a2ee8aaa
2 changed files with 94 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/core/tcp_in.c
View File

@ -161,34 +161,51 @@ tcp_input(struct pbuf *p, struct netif *inp)
tcphdr_optlen = tcphdr_opt1len = (hdrlen * 4) - TCP_HLEN;
tcphdr_opt2 = NULL;
if (p->len < hdrlen * 4) {
if (p->len >= TCP_HLEN) {
if (p->len >= TCP_HLEN && p->next != NULL) {
/* TCP header fits into first pbuf, options don't - data is in the next pbuf */
u16_t optlen = tcphdr_opt1len;
pbuf_header(p, -TCP_HLEN); /* cannot fail */
LWIP_ASSERT("tcphdr_opt1len >= p->len", tcphdr_opt1len >= p->len);
LWIP_ASSERT("p->next != NULL", p->next != NULL);
LWIP_ASSERT("tcphdr_opt1len >= p->len", tcphdr_opt1len >= p->len); /* that would be a programming error */
tcphdr_opt1len = p->len;
if (optlen > tcphdr_opt1len) {
s16_t opt2len;
/* options continue in the next pbuf: set p to zero length and hide the
options in the next pbuf (adjusting p->tot_len) */
u8_t phret = pbuf_header(p, -(s16_t)tcphdr_opt1len);
LWIP_ASSERT("phret == 0", phret == 0);
if (phret != 0) {
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: pbuf_header failed, illegal tcphdr_opt1len (%"U16_F")\n", tcphdr_opt1len));
TCP_STATS_INC(tcp.lenerr);
goto dropped;
}
if(tcphdr_optlen - tcphdr_opt1len > p->tot_len) {
/* drop short packets */
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: short packet (%"U16_F" bytes) discarded\n", p->tot_len));
TCP_STATS_INC(tcp.lenerr);
goto dropped;
}
tcphdr_opt2 = (u8_t*)p->next->payload;
opt2len = optlen - tcphdr_opt1len;
phret = pbuf_header(p->next, -opt2len);
LWIP_ASSERT("phret == 0", phret == 0);
if (phret != 0) {
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: pbuf_header failed, illegal opt2len (%"U16_F")\n", opt2len));
TCP_STATS_INC(tcp.lenerr);
goto dropped;
}
/* p->next->payload now points to the TCP data */
/* manually adjust p->tot_len to changed p->next->tot_len change */
p->tot_len -= opt2len;
}
LWIP_ASSERT("p->len == 0", p->len == 0);
if (p->len != 0) {
/* drop malformed packets */
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: malformed packet (p->len %"U16_F" != 0), discarded\n", p->len));
TCP_STATS_INC(tcp.lenerr);
goto dropped;
}
} else {
/* drop short packets */
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: short packet\n"));

71
test/unit/tcp/test_tcp.c
View File

@ -3,6 +3,7 @@
#include "lwip/priv/tcp_priv.h"
#include "lwip/stats.h"
#include "tcp_helper.h"
#include "lwip/inet_chksum.h"
#ifdef _MSC_VER
#pragma warning(disable: 4307) /* we explicitly wrap around TCP seqnos */
@ -122,6 +123,75 @@ START_TEST(test_tcp_recv_inseq)
}
END_TEST
/** Check that we handle malformed tcp headers, and discard the pbuf(s) */
START_TEST(test_tcp_malformed_header)
{
struct test_tcp_counters counters;
struct tcp_pcb* pcb;
struct pbuf* p;
char data[] = {1, 2, 3, 4};
ip_addr_t remote_ip, local_ip, netmask;
u16_t data_len, chksum;
u16_t remote_port = 0x100, local_port = 0x101;
struct netif netif;
struct test_tcp_txcounters txcounters;
struct tcp_hdr *hdr;
LWIP_UNUSED_ARG(_i);
/* initialize local vars */
memset(&netif, 0, sizeof(netif));
IP_ADDR4(&local_ip, 192, 168, 1, 1);
IP_ADDR4(&remote_ip, 192, 168, 1, 2);
IP_ADDR4(&netmask, 255, 255, 255, 0);
test_tcp_init_netif(&netif, &txcounters, &local_ip, &netmask);
data_len = sizeof(data);
/* initialize counter struct */
memset(&counters, 0, sizeof(counters));
counters.expected_data_len = data_len;
counters.expected_data = data;
/* create and initialize the pcb */
pcb = test_tcp_new_counters_pcb(&counters);
EXPECT_RET(pcb != NULL);
tcp_set_state(pcb, ESTABLISHED, &local_ip, &remote_ip, local_port, remote_port);
/* create a segment */
p = tcp_create_rx_segment(pcb, counters.expected_data, data_len, 0, 0, 0);
pbuf_header(p, -(s16_t)sizeof(struct ip_hdr));
hdr = (struct tcp_hdr *)p->payload;
TCPH_HDRLEN_FLAGS_SET(hdr, 15, 0x3d1);
hdr->chksum = 0;
chksum = ip_chksum_pseudo(p, IP_PROTO_TCP, p->tot_len,
&remote_ip, &local_ip);
hdr->chksum = chksum;
pbuf_header(p, sizeof(struct ip_hdr));
EXPECT(p != NULL);
EXPECT(p->next == NULL);
if (p != NULL) {
/* pass the segment to tcp_input */
test_tcp_input(p, &netif);
/* check if counters are as expected */
EXPECT(counters.close_calls == 0);
EXPECT(counters.recv_calls == 0);
EXPECT(counters.recved_bytes == 0);
EXPECT(counters.err_calls == 0);
}
/* make sure the pcb is freed */
EXPECT(lwip_stats.memp[MEMP_TCP_PCB].used == 1);
tcp_abort(pcb);
EXPECT(lwip_stats.memp[MEMP_TCP_PCB].used == 0);
}
END_TEST
/** Provoke fast retransmission by duplicate ACKs and then recover by ACKing all sent data.
* At the end, send more data. */
START_TEST(test_tcp_fast_retx_recover)
@ -661,6 +731,7 @@ tcp_suite(void)
testfunc tests[] = {
TESTFUNC(test_tcp_new_abort),
TESTFUNC(test_tcp_recv_inseq),
TESTFUNC(test_tcp_malformed_header),
TESTFUNC(test_tcp_fast_retx_recover),
TESTFUNC(test_tcp_fast_rexmit_wraparound),
TESTFUNC(test_tcp_rto_rexmit_wraparound),