Several Security Issues Report

[GANGE666]

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.

picotcp/modules/pico_icmp4.c

Lines 375 to 400 in 72ffa74

	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.

picotcp/stack/pico_socket.c

Lines 1595 to 1642 in 72ffa74

	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;
	}

[danielinux]

Thanks for reporting this, I'll have a look ASAP