Hi, I've found several potential issues in picoTCP v1.7.0 and picoTCP-NG v2.1. While it's difficult for these bugs to actually have an impact, I still think it's worth letting you know about and fixing these potential issues.
1. Integer Overflow in pico_icmp4_send_echo
The parameter cookie of function pico_icmp4_send_echo is completely controllable by the developer. When the cookie->size is set to a large value, an integer overflow will occur when calculating transport_len (Line 393). PICO_ICMPHDR_UN_SIZE is specified as 8, so overflow occurs when the value of cookie->size larger than 65528.
If developers use PicoTCP to develop applications and allow remote visitors to set cookie->size, it may lead to out-of-bounds read and write, which may eventually lead to information leakage and even remote code execution.
|
static int8_t pico_icmp4_send_echo(struct pico_stack *S, struct pico_icmp4_ping_cookie *cookie) |
|
{ |
|
struct pico_frame *echo = NULL; |
|
struct pico_icmp4_hdr *hdr; |
|
struct pico_device *dev = pico_ipv4_source_dev_find(S, &cookie->dst); |
|
if (!dev) |
|
return -1; |
|
|
|
echo = pico_proto_ipv4.alloc(S, &pico_proto_ipv4, dev, (uint16_t)(PICO_ICMPHDR_UN_SIZE + cookie->size)); |
|
if (!echo) |
|
return -1; |
|
|
|
hdr = (struct pico_icmp4_hdr *) echo->transport_hdr; |
|
|
|
hdr->type = PICO_ICMP_ECHO; |
|
hdr->code = 0; |
|
hdr->hun.ih_idseq.idseq_id = short_be(cookie->id); |
|
hdr->hun.ih_idseq.idseq_seq = short_be(cookie->seq); |
|
echo->transport_len = (uint16_t)(PICO_ICMPHDR_UN_SIZE + cookie->size); |
|
echo->payload = echo->transport_hdr + PICO_ICMPHDR_UN_SIZE; |
|
echo->payload_len = cookie->size; |
|
/* XXX: Fill payload */ |
|
pico_icmp4_checksum(echo); |
|
pico_ipv4_frame_push(S, echo, &cookie->dst, PICO_PROTO_ICMP4); |
|
return 0; |
|
} |
2. Integer Overflow in pico_socket_fionread
I think there are two potential integer overflows in the pico_socket_fionread function. When a packet that is too short is received, an integer underflow may occur when calculating f->payload_len of the received packet. This issue may occur when UDP packets are less than 8 bytes ([Line 1619]) or IPV4 packets are less than 20 bytes ([Line 1633). (just like CVE-2020-17443)
Although I didn't find where to call pico_socket_fionread, but to be on the safe side, I hope you can fix both issues.
|
int pico_socket_fionread(struct pico_socket *s) |
|
{ |
|
|
|
struct pico_frame *f; |
|
if (s == NULL) { |
|
pico_err = PICO_ERR_EINVAL; |
|
return -1; |
|
} else { |
|
if (pico_check_socket(s) != 0) { |
|
pico_err = PICO_ERR_EINVAL; |
|
return -1; |
|
} |
|
} |
|
if ((s->state & PICO_SOCKET_STATE_BOUND) == 0) { |
|
pico_err = PICO_ERR_EIO; |
|
return -1; |
|
} |
|
if (PROTO(s) == PICO_PROTO_UDP) |
|
{ |
|
f = pico_queue_peek(&s->q_in); |
|
if (!f) |
|
return 0; |
|
if(!f->payload_len) { |
|
f->payload = f->transport_hdr + sizeof(struct pico_udp_hdr); |
|
f->payload_len = (uint16_t)(f->transport_len - sizeof(struct pico_udp_hdr)); |
|
} |
|
return f->payload_len; |
|
} |
|
#ifdef PICO_SUPPORT_RAWSOCKETS |
|
else if (PROTO(s) == PICO_PROTO_IPV4) { |
|
struct pico_socket_ipv4 *s4 = (struct pico_socket_ipv4 *)s; |
|
f = pico_queue_peek(&s->q_in); |
|
if (!f) |
|
return 0; |
|
if(!f->payload_len) { |
|
f->payload = f->net_hdr + f->net_len; |
|
f->payload_len = (uint16_t)(f->len); |
|
if (!s4->hdr_included) |
|
f->payload_len = (uint16_t)(f->payload_len - PICO_SIZE_IP4HDR); |
|
} |
|
return f->payload_len; |
|
} |
|
#endif |
|
else if (PROTO(s) == PICO_PROTO_TCP) { |
|
return pico_tcp_queue_in_size(s); |
|
} |
|
return 0; |
|
} |