diff --git a/docs/security.md b/docs/security.md index 81d6f998..9c5ebeb7 100644 --- a/docs/security.md +++ b/docs/security.md @@ -9,14 +9,7 @@ Please consider checking the list of known issues before using picoTCP in produc Vulnerabilities that can be found in picoTCP-NG v.2.1: -* ❗ Improper bound checking against the parsing of domain names may result in remote code execution (CVE-2020-24338) - * Triaged. Awaiting assignment. - -* ❗ Improper bound checking against the parsing of domain names may result in a denial of service (CVE-2020-24339) - * Triaged. Awaiting assignment. - -* ❗ Improper checks in the process of DNS response handling which may lead to memory corruption (CVE-2020-24340) - * Triaged. Awaiting assignment. +* None currently known. Vulnerabilities fixed in this version: @@ -41,3 +34,27 @@ Vulnerabilities fixed in this version: * Improper checks against the length of incoming TCP packets may enable an out-of-bound read and/or memory corruption (CVE-2020-24341) * **Fixed in v2.1** :heavy_check_mark: +* Improper checks in the process of DNS response handling which may lead to memory corruption (CVE-2020-24340) + * **Fixed in 2.2** :heavy_check_mark: + +* Improper bound checking against the parsing of domain names which may result in remote code execution (CVE-2020-24338) + * **Fixed in 2.2** :heavy_check_mark: + +* Improper bound checking against the parsing of domain names which may result in a denial of service (CVE-2020-24339) + * **Fixed in 2.2** :heavy_check_mark: + +* Heap buffer overflow in IP fragment reassembly due to a 16-bit length truncation (Forescout FSCT-2026-0017) + * **Fixed in 2.2** :heavy_check_mark: + +* Address-buffer overflow in the pico_socket_accept() peer-address copy (Forescout FSCT-2026-0018) + * **Fixed in 2.2** :heavy_check_mark: + +* Unsigned integer underflow in UDP receive leading to an out-of-bounds read (Forescout FSCT-2026-0019) + * **Fixed in 2.2** :heavy_check_mark: + +* Out-of-bounds read in DNS client name lookup due to an unbounded name walk (Forescout FSCT-2026-0020) + * **Fixed in 2.2** :heavy_check_mark: + +* Out-of-bounds read in IPv4 input processing due to an off-by-option-length bound check (Forescout FSCT-2026-0022) + * **Fixed in 2.2** :heavy_check_mark: + diff --git a/modules/pico_6lowpan.c b/modules/pico_6lowpan.c index d53234b6..b3395cf5 100644 --- a/modules/pico_6lowpan.c +++ b/modules/pico_6lowpan.c @@ -104,6 +104,9 @@ #define FRAG1_DISPATCH (0xC0) #define FRAGN_DISPATCH (0xE0) #define FRAG_TIMEOUT (5) +/* Cap on concurrent reassembly entries to bound memory use against a + * flood of distinct fragment tags. */ +#define PICO_6LOWPAN_MAX_REASSEMBLY (16) /******************************************************************************* * Type definitions ******************************************************************************/ @@ -1069,6 +1072,10 @@ pico_iphc_reassemble(struct pico_frame *f, uint8_t **chunks, int32_t *chunks_len /* Calculate buffer size including IPv6 payload */ payload_len = (int32_t)f->len - handled_len; + if (payload_len < 0) { + /* Compressed length exceeds the frame; malformed. */ + return pico_iphc_bail_out(chunks, n); + } processed_len += payload_len; /* Length of frame after processing */ /* Reallocate frame size if there isn't enough room available */ @@ -1276,6 +1283,13 @@ frag_update(struct pico_frame *f, struct frag_ctx *frag, uint8_t units, uint16_t static void frag_fill(uint8_t *frag, uint8_t dispatch, uint16_t dgram_size, uint16_t tag, uint8_t dgram_off, int32_t offset, uint16_t copy, uint16_t copied, uint8_t *buf) { + /* Never copy past the end of the datagram. */ + if ((uint32_t)copied + (uint32_t)copy > (uint32_t)dgram_size) { + if (copied >= dgram_size) + copy = 0; + else + copy = (uint16_t)(dgram_size - copied); + } frag[0] = (uint8_t)(dispatch | ((uint8_t)short_be(dgram_size) & 0x07)); frag[1] = (uint8_t)(short_be(dgram_size) >> 8); frag[2] = (uint8_t)(short_be(tag)); @@ -1525,8 +1539,40 @@ pico_6lowpan_decompress(struct pico_frame *f) static int32_t defrag_new(struct pico_frame *f, uint16_t dgram_size, uint16_t tag, uint16_t off) { - struct pico_frame *r = pico_proto_6lowpan_ll.alloc(f->dev->stack, &pico_proto_6lowpan_ll, f->dev, dgram_size); + struct pico_frame *r; + struct pico_stack *S = f->dev->stack; + struct pico_tree_node *i = NULL, *next = NULL; + struct frag_ctx *key = NULL; + struct frag_ctx *oldest = NULL; + int32_t count = 0; + + /* Cap concurrent reassembly entries to bound memory use. */ + pico_tree_foreach_safe(i, &S->LPReassemblyTree, next) { + if ((key = i->keyValue)) { + count++; + if (!oldest || key->timestamp < oldest->timestamp) { + oldest = key; + } + } + } + if (count >= PICO_6LOWPAN_MAX_REASSEMBLY) { + /* Tree is full; evict the oldest reassembly to make room. */ + if (oldest) { + pico_tree_delete(&S->LPReassemblyTree, oldest); + pico_frame_discard(oldest->f); + PICO_FREE(oldest); + } else { + return -1; + } + } + + r = pico_proto_6lowpan_ll.alloc(S, &pico_proto_6lowpan_ll, f->dev, dgram_size); if (r) { + if ((uint32_t)off + (uint32_t)f->len > (uint32_t)dgram_size) { + pico_frame_discard(f); + pico_frame_discard(r); + return -1; + } r->start = r->buffer + (int32_t)(r->buffer_len - (uint32_t)dgram_size); r->len = dgram_size; r->net_hdr = r->start; @@ -1535,7 +1581,7 @@ defrag_new(struct pico_frame *f, uint16_t dgram_size, uint16_t tag, uint16_t off r->src = f->src; r->dst = f->dst; buf_move(r->net_hdr + off, f->start, f->len); - if (frag_store(r, dgram_size, tag, 0, (uint16_t)f->len, &f->dev->stack->LPReassemblyTree) < 0) { + if (frag_store(r, dgram_size, tag, 0, (uint16_t)f->len, &S->LPReassemblyTree) < 0) { pico_frame_discard(f); pico_frame_discard(r); return -1; @@ -1549,6 +1595,10 @@ static int32_t defrag_update(struct frag_ctx *frag, uint16_t off, struct pico_frame *f) { struct pico_frame *r = frag->f; + if ((uint32_t)off + (uint32_t)f->len > (uint32_t)frag->dgram_size) { + pico_frame_discard(f); + return -1; + } buf_move(r->start + (int32_t)off, f->start, f->len); /* Copy at start */ frag->copied = (uint16_t)(frag->copied + (uint16_t)f->len); pico_frame_discard(f); diff --git a/modules/pico_6lowpan_ll.c b/modules/pico_6lowpan_ll.c index 2411b98b..dc0b9e2e 100644 --- a/modules/pico_6lowpan_ll.c +++ b/modules/pico_6lowpan_ll.c @@ -349,7 +349,7 @@ pico_6lowpan_ll_process_in(struct pico_stack *S, struct pico_protocol *self, str case FRAME_6LOWPAN_LL_DISCARD: /* Something went wrong, discard the frame */ pico_frame_discard(f); - break; + return -1; default: /* Success, update link layer header length */ len = (uint32_t)(len + (uint32_t)ret); diff --git a/modules/pico_802154.c b/modules/pico_802154.c index fd58110c..29ddcadd 100644 --- a/modules/pico_802154.c +++ b/modules/pico_802154.c @@ -202,8 +202,22 @@ pico_802154_process_in(struct pico_frame *f) { #ifndef PICO_6LOWPAN_NOMAC struct pico_802154_hdr *hdr = (struct pico_802154_hdr *)f->net_hdr; - uint16_t fcf = short_be(hdr->fcf); - uint8_t hlen = 0; + uint16_t fcf; + uint8_t hlen; + + /* The frame must hold at least the minimum 802.15.4 header before any + * field is read from it. */ + if (f->len < SIZE_802154_MHR_MIN) { + return FRAME_6LOWPAN_LL_DISCARD; + } + + fcf = short_be(hdr->fcf); + hlen = frame_802154_hdr_len(hdr); + if (f->len < (uint32_t)hlen) { + /* Frame shorter than its own header; malformed. */ + return FRAME_6LOWPAN_LL_DISCARD; + } + f->src.pan = frame_802154_src(hdr); f->dst.pan = frame_802154_dst(hdr); @@ -214,8 +228,6 @@ pico_802154_process_in(struct pico_frame *f) f->flags |= PICO_FRAME_FLAG_LL_SEC; } - hlen = frame_802154_hdr_len(hdr); - /* XXX: Generic procedure to move forward in incoming processing function * is updating the net_hdr-pointer */ f->net_hdr = f->datalink_hdr + (int32_t)hlen; diff --git a/modules/pico_dev_ppp.c b/modules/pico_dev_ppp.c index ccaa3f28..7d2f5200 100644 --- a/modules/pico_dev_ppp.c +++ b/modules/pico_dev_ppp.c @@ -1826,7 +1826,7 @@ static void auth_rsp(struct pico_device_ppp *ppp) uint8_t *md5resp = resp + PPP_HDR_SIZE + PPP_PROTO_SLOT_SIZE + sizeof(struct pico_chap_hdr) + 1; uint8_t *md5resp_len = resp + PPP_HDR_SIZE + PPP_PROTO_SLOT_SIZE + sizeof(struct pico_chap_hdr); uint8_t *challenge; - uint32_t i = 0, pwdlen; + uint32_t i = 0, pwdlen, recvd_len; uint8_t *recvd_challenge_len = ppp->pkt + sizeof(struct pico_chap_hdr); uint8_t *recvd_challenge = recvd_challenge_len + 1; size_t challenge_size = CHALLENGE_SIZE(ppp, ch); @@ -1841,8 +1841,13 @@ static void auth_rsp(struct pico_device_ppp *ppp) challenge[i++] = ch->id; memcpy(challenge + i, ppp->password, pwdlen); i += pwdlen; - memcpy(challenge + i, recvd_challenge, *recvd_challenge_len); - i += *recvd_challenge_len; + /* The buffer is sized by ch->len; the on-wire challenge length is an + * independent field, so cap the copy to the remaining buffer space. */ + recvd_len = *recvd_challenge_len; + if (recvd_len > challenge_size - i) + recvd_len = (uint32_t)(challenge_size - i); + memcpy(challenge + i, recvd_challenge, recvd_len); + i += recvd_len; pico_md5sum(md5resp, challenge, i); PICO_FREE(challenge); rh->id = ch->id; diff --git a/modules/pico_dev_radiotest.c b/modules/pico_dev_radiotest.c index 2e2554a0..f81adad8 100644 --- a/modules/pico_dev_radiotest.c +++ b/modules/pico_dev_radiotest.c @@ -329,6 +329,8 @@ static int radiotest_poll(struct pico_device *dev, int loop_score) if (p.revents & POLLIN) { ret_len = (int)recv(connection, &phy, (size_t)1, 0); if (ret_len != 1) return loop_score; + if (phy > sizeof(buf)) + return loop_score; ret_len = (int)recv(connection, buf, (size_t)phy, 0); if (ret_len != (int)phy) return loop_score; @@ -338,7 +340,7 @@ static int radiotest_poll(struct pico_device *dev, int loop_score) } } - if (ret_len < 2) { /* Not valid */ + if (ret_len < 4) { /* Not a valid frame (frame + CRC + address) */ return loop_score; } diff --git a/modules/pico_dhcp_client.c b/modules/pico_dhcp_client.c index c4ff339b..a84756f9 100644 --- a/modules/pico_dhcp_client.c +++ b/modules/pico_dhcp_client.c @@ -496,31 +496,43 @@ static void pico_dhcp_client_recv_params(struct pico_dhcp_client_cookie *dhcpc, break; case PICO_DHCP_OPT_MSGTYPE: + if (opt->len < 1) + break; dhcpc->event = opt->ext.msg_type.type; dhcpc_dbg("DHCP client: message type %u\n", dhcpc->event); break; case PICO_DHCP_OPT_LEASETIME: + if (opt->len < 4) + break; dhcpc->lease_time = long_be(opt->ext.lease_time.time); dhcpc_dbg("DHCP client: lease time %u\n", dhcpc->lease_time); break; case PICO_DHCP_OPT_RENEWALTIME: + if (opt->len < 4) + break; dhcpc->t1_time = long_be(opt->ext.renewal_time.time); dhcpc_dbg("DHCP client: renewal time %u\n", dhcpc->t1_time); break; case PICO_DHCP_OPT_REBINDINGTIME: + if (opt->len < 4) + break; dhcpc->t2_time = long_be(opt->ext.rebinding_time.time); dhcpc_dbg("DHCP client: rebinding time %u\n", dhcpc->t2_time); break; case PICO_DHCP_OPT_ROUTER: + if (opt->len < 4) + break; dhcpc->gateway = opt->ext.router.ip; dhcpc_dbg("DHCP client: router %08X\n", dhcpc->gateway.addr); break; case PICO_DHCP_OPT_DNS: + if (opt->len < 4) + break; dhcpc->nameserver[0] = opt->ext.dns1.ip; dhcpc_dbg("DHCP client: dns1 %08X\n", dhcpc->nameserver[0].addr); if (opt->len >= 8) { @@ -531,11 +543,15 @@ static void pico_dhcp_client_recv_params(struct pico_dhcp_client_cookie *dhcpc, break; case PICO_DHCP_OPT_NETMASK: + if (opt->len < 4) + break; dhcpc->netmask = opt->ext.netmask.ip; dhcpc_dbg("DHCP client: netmask %08X\n", dhcpc->netmask.addr); break; case PICO_DHCP_OPT_SERVERID: + if (opt->len < 4) + break; dhcpc->server_id = opt->ext.server_id.ip; dhcpc_dbg("DHCP client: server ID %08X\n", dhcpc->server_id.addr); break; @@ -546,21 +562,23 @@ static void pico_dhcp_client_recv_params(struct pico_dhcp_client_cookie *dhcpc, case PICO_DHCP_OPT_HOSTNAME: { - uint32_t maxlen = PICO_DHCP_HOSTNAME_MAXLEN; + uint32_t maxlen = PICO_DHCP_HOSTNAME_MAXLEN - 1; if (opt->len < maxlen) maxlen = opt->len; strncpy(dhcpc->stack->dhcpc_host_name, opt->ext.string.txt, maxlen); + dhcpc->stack->dhcpc_host_name[maxlen] = '\0'; } break; case PICO_DHCP_OPT_DOMAINNAME: { - uint32_t maxlen = PICO_DHCP_HOSTNAME_MAXLEN; + uint32_t maxlen = PICO_DHCP_HOSTNAME_MAXLEN - 1; if (opt->len < maxlen) maxlen = opt->len; strncpy(dhcpc->stack->dhcpc_domain_name, opt->ext.string.txt, maxlen); + dhcpc->stack->dhcpc_domain_name[maxlen] = '\0'; } break; diff --git a/modules/pico_dhcp_server.c b/modules/pico_dhcp_server.c index 911e6378..622ea6c6 100644 --- a/modules/pico_dhcp_server.c +++ b/modules/pico_dhcp_server.c @@ -50,6 +50,10 @@ /* maximum size of a DHCP message */ #define DHCP_SERVER_MAXMSGSIZE (PICO_IP_MRU - sizeof(struct pico_ipv4_hdr) - sizeof(struct pico_udp_hdr)) +/* Cap on concurrent in-flight negotiations: a flood of DISCOVERs with distinct + * xids must not grow the negotiation tree without bound. */ +#define PICO_DHCP_SERVER_MAX_NEGOTIATIONS 16 + enum dhcp_server_state { PICO_DHCP_STATE_DISCOVER = 0, PICO_DHCP_STATE_OFFER, @@ -224,10 +228,23 @@ static struct pico_dhcp_server_negotiation *pico_dhcp_server_add_negotiation(str struct pico_dhcp_server_setting test = { 0 }; + struct pico_tree_node *node, *victim; + uint32_t count = 0; if (pico_dhcp_server_find_negotiation(dev->stack, hdr->xid)) return NULL; + /* Cap concurrent negotiations; evict the lowest-xid entry when full + * (the tree is keyed by xid, so firstNode is the smallest xid). */ + pico_tree_foreach(node, &dev->stack->DHCPNegotiations) { + count++; + } + if (count >= PICO_DHCP_SERVER_MAX_NEGOTIATIONS) { + victim = pico_tree_firstNode(dev->stack->DHCPNegotiations.root); + if (victim != &LEAF) + PICO_FREE(pico_tree_delete(&dev->stack->DHCPNegotiations, victim->keyValue)); + } + dhcpn = PICO_ZALLOC(sizeof(struct pico_dhcp_server_negotiation)); if (!dhcpn) { pico_err = PICO_ERR_ENOMEM; @@ -431,6 +448,8 @@ int pico_dhcp_server_destroy(struct pico_device *dev) struct pico_dhcp_server_setting *found, test = { 0 }; + struct pico_tree_node *node, *next; + struct pico_dhcp_server_negotiation *n; test.dev = dev; found = pico_tree_findKey(&dev->stack->DHCPSettings, &test); if (!found) { @@ -440,6 +459,15 @@ int pico_dhcp_server_destroy(struct pico_device *dev) pico_tree_delete(&dev->stack->DHCPSettings, found); PICO_FREE(found); + + /* Free in-flight negotiations belonging to this device only, so that + * destroying one server does not disrupt others on the same stack. + * pico_tree_delete frees each tree node. */ + pico_tree_foreach_safe(node, &dev->stack->DHCPNegotiations, next) { + n = node->keyValue; + if (n->dhcps && n->dhcps->dev == dev) + PICO_FREE(pico_tree_delete(&dev->stack->DHCPNegotiations, node->keyValue)); + } return 0; } diff --git a/modules/pico_dns_client.c b/modules/pico_dns_client.c index 6a7600b5..4beaa9c6 100644 --- a/modules/pico_dns_client.c +++ b/modules/pico_dns_client.c @@ -341,6 +341,7 @@ static int pico_dns_client_check_asuffix(struct pico_dns_record_suffix *suf, str static char *pico_dns_client_seek_suffix(char *suf, struct pico_dns_header *pre, struct pico_dns_query *q) { struct pico_dns_record_suffix *asuffix = NULL; + const char *end = (const char *)pre + PICO_IP_MRU; uint16_t comp = 0, compression = 0; uint16_t i = 0; @@ -348,6 +349,8 @@ static char *pico_dns_client_seek_suffix(char *suf, struct pico_dns_header *pre, return NULL; while (i++ < short_be(pre->ancount)) { + if (suf + sizeof(uint16_t) > end) + return NULL; comp = short_from(suf); compression = short_be(comp); switch (compression >> 14) { @@ -355,6 +358,8 @@ static char *pico_dns_client_seek_suffix(char *suf, struct pico_dns_header *pre, while (compression >> 14 == PICO_DNS_POINTER) { dns_dbg("DNS: pointer\n"); suf += sizeof(uint16_t); + if (suf + sizeof(uint16_t) > end) + return NULL; comp = short_from(suf); compression = short_be(comp); } @@ -362,7 +367,9 @@ static char *pico_dns_client_seek_suffix(char *suf, struct pico_dns_header *pre, case PICO_DNS_LABEL: dns_dbg("DNS: label\n"); - suf = pico_dns_client_seek(suf, (const char *)pre + PICO_IP_MRU); + suf = pico_dns_client_seek(suf, end); + if (!suf) + return NULL; break; default: @@ -370,11 +377,16 @@ static char *pico_dns_client_seek_suffix(char *suf, struct pico_dns_header *pre, return NULL; } + if (suf + sizeof(struct pico_dns_record_suffix) > end) + return NULL; asuffix = (struct pico_dns_record_suffix *)suf; if (!asuffix) break; if (pico_dns_client_check_asuffix(asuffix, q) < 0) { + if (suf + sizeof(struct pico_dns_record_suffix) + + short_be(asuffix->rdlength) > end) + return NULL; suf += (sizeof(struct pico_dns_record_suffix) + short_be(asuffix->rdlength)); continue; } @@ -558,7 +570,7 @@ static void pico_dns_try_fallback_cname(struct pico_dns_query *q, struct pico_dn /* Found CNAME response. Re-initiating query. */ asuffix = (struct pico_dns_record_suffix *)p_asuffix; - cname = pico_dns_decompress_name((char *)asuffix + sizeof(struct pico_dns_record_suffix), (pico_dns_packet *)h); /* allocates memory! */ + cname = pico_dns_decompress_name((char *)asuffix + sizeof(struct pico_dns_record_suffix), (pico_dns_packet *)h, PICO_IP_MRU); /* allocates memory! */ cname_orig = cname; /* to free later */ if (cname == NULL) diff --git a/modules/pico_dns_common.c b/modules/pico_dns_common.c index 2bdd13f5..5561d86f 100644 --- a/modules/pico_dns_common.c +++ b/modules/pico_dns_common.c @@ -105,50 +105,86 @@ pico_dns_namelen_comp(char *name) * Returns the uncompressed name in DNS name format when DNS name compression * is applied to the packet-buffer. * - * @param name Compressed name, should be in the bounds of the actual packet - * @param packet Packet that contains the compressed name + * @param name Compressed name, must lie within [packet, packet + packet_len) + * @param packet Packet that contains the compressed name + * @param packet_len Length of the packet in bytes * @return Returns the decompressed name, NULL on failure. * ****************************************************************************/ char * -pico_dns_decompress_name(char *name, pico_dns_packet *packet) +pico_dns_decompress_name(char *name, pico_dns_packet *packet, size_t packet_len) { char decompressed_name[PICO_DNS_NAMEBUF_SIZE] = { 0 }; char *return_name = NULL; + const uint8_t *p = (const uint8_t *)name; + const uint8_t *end = (const uint8_t *)packet + packet_len; uint16_t ptr = 0, nslen = 0; uint16_t decompressed_index = 0; - char *label = NULL, *next = NULL; - /* Reading labels until reaching to pointer or NULL terminator. - * Only one pointer is allowed in DNS compression, the pointer is always the last according to the RFC */ - dns_name_foreach_label_safe(label, name, next, PICO_DNS_NAMEBUF_SIZE) { + /* Check params: the name must be fully inside the packet */ + if (!name || !packet || ((const uint8_t *)name) < (const uint8_t *)packet || + ((const uint8_t *)name) + 1 > end) { + pico_err = PICO_ERR_EINVAL; + return NULL; + } + + /* Walk labels from the start of the name until a terminator or a + * compression pointer. Every read is bounded by the packet end. */ + while (p < end) { + uint8_t c = *p; - uint8_t label_size = (uint8_t)(*label + 1); - if (decompressed_index + label_size >= PICO_DNS_NAMEBUF_SIZE) { + if (c == 0x00 || c >= 0xC0) { + break; + } + if (c > 63 || p + c + 1 > end) { return NULL; } - memcpy(&decompressed_name[decompressed_index], label, label_size); - decompressed_index = (uint16_t)(decompressed_index + label_size); + if (decompressed_index + c + 1 >= PICO_DNS_NAMEBUF_SIZE) { + return NULL; + } + memcpy(&decompressed_name[decompressed_index], p, (size_t)(c + 1)); + decompressed_index = (uint16_t)(decompressed_index + c + 1); + p += c + 1; } - - if (decompressed_index >= PICO_DNS_NAMEBUF_SIZE) { + /* The walk must have stopped on a terminator or a pointer, not run off + * the end of the packet. */ + if (p >= end) { return NULL; } - if (*label & 0xC0) { - /* Found compression bits */ - ptr = (uint16_t)((((uint16_t) *label) & 0x003F) << 8); - ptr = (uint16_t)(ptr | (uint16_t) *(label + 1)); - label = (char *)((uint8_t *)packet + ptr); + if (*p >= 0xC0) { + /* Compression pointer: both bytes and the target offset must lie + * inside the packet. */ + if (p + 2 > end) { + return NULL; + } + ptr = (uint16_t)((((uint16_t)*p & 0x003F) << 8) | (uint16_t)*(p + 1)); + if (ptr >= packet_len) { + return NULL; + } + p = (const uint8_t *)packet + ptr; + + /* Walk the remainder from the pointer target to its terminator. A + * second pointer is not valid DNS compression. */ + while (p < end) { + uint8_t c = *p; - dns_name_foreach_label_safe(label, label, next, PICO_DNS_NAMEBUF_SIZE - decompressed_index) { - uint8_t label_size = (uint8_t)(*label + 1); - if (decompressed_index + label_size >= PICO_DNS_NAMEBUF_SIZE) { + if (c == 0x00) { + break; + } + if (c >= 0xC0 || c > 63 || p + c + 1 > end) { + return NULL; + } + if (decompressed_index + c + 1 >= PICO_DNS_NAMEBUF_SIZE) { return NULL; } - memcpy(&decompressed_name[decompressed_index], label, label_size); - decompressed_index = (uint16_t) (decompressed_index + label_size); + memcpy(&decompressed_name[decompressed_index], p, (size_t)(c + 1)); + decompressed_index = (uint16_t)(decompressed_index + c + 1); + p += c + 1; + } + if (p >= end) { + return NULL; } } @@ -687,16 +723,17 @@ pico_dns_question_create(const char *url, * * @param question Question you want to decompress the name of * @param packet Packet in which the DNS question is contained. + * @param packet_len Length of the packet in bytes * @return Pointer to original name of the DNS question before decompressing. * ****************************************************************************/ char * pico_dns_question_decompress(struct pico_dns_question *question, - pico_dns_packet *packet) + pico_dns_packet *packet, size_t packet_len) { char *qname_original = question->qname; /* Try to decompress the question name */ - question->qname = pico_dns_decompress_name(question->qname, packet); + question->qname = pico_dns_decompress_name(question->qname, packet, packet_len); return qname_original; } @@ -1006,16 +1043,17 @@ pico_dns_record_create(struct pico_stack *S, * * @param record DNS record to decompress the name of. * @param packet Packet in which is DNS record is present + * @param packet_len Length of the packet in bytes * @return Pointer to original name of the DNS record before decompressing. * ****************************************************************************/ char * pico_dns_record_decompress(struct pico_dns_record *record, - pico_dns_packet *packet) + pico_dns_packet *packet, size_t packet_len) { char *rname_original = record->rname; /* Try to decompress the record name */ - record->rname = pico_dns_decompress_name(record->rname, packet); + record->rname = pico_dns_decompress_name(record->rname, packet, packet_len); return rname_original; } diff --git a/modules/pico_dns_common.h b/modules/pico_dns_common.h index 0ad4c616..1d042ea3 100644 --- a/modules/pico_dns_common.h +++ b/modules/pico_dns_common.h @@ -168,12 +168,13 @@ pico_dns_namelen_comp(char *name); * Returns the uncompressed name in DNS name format when DNS name compression * is applied to the packet-buffer. * - * @param name Compressed name, should be in the bounds of the actual packet - * @param packet Packet that contains the compressed name + * @param name Compressed name, must lie within [packet, packet + packet_len) + * @param packet Packet that contains the compressed name + * @param packet_len Length of the packet in bytes * @return Returns the decompressed name, NULL on failure. * ****************************************************************************/ char * -pico_dns_decompress_name(char *name, pico_dns_packet *packet); +pico_dns_decompress_name(char *name, pico_dns_packet *packet, size_t packet_len); /* **************************************************************************** * Converts a DNS name in DNS name format to a name in URL format. Provides @@ -314,11 +315,12 @@ pico_dns_question_create(const char *url, * * @param question Question you want to decompress the name of * @param packet Packet in which the DNS question is contained. + * @param packet_len Length of the packet in bytes * @return Pointer to original name of the DNS question before decompressing. * ****************************************************************************/ char * pico_dns_question_decompress(struct pico_dns_question *question, - pico_dns_packet *packet); + pico_dns_packet *packet, size_t packet_len); /* MARK: RESOURCE RECORD FUNCTIONS */ @@ -373,11 +375,12 @@ pico_dns_record_create(struct pico_stack *S, * * @param record DNS record to decompress the name of. * @param packet Packet in which is DNS record is present + * @param packet_len Length of the packet in bytes * @return Pointer to original name of the DNS record before decompressing. * ****************************************************************************/ char * pico_dns_record_decompress(struct pico_dns_record *record, - pico_dns_packet *packet); + pico_dns_packet *packet, size_t packet_len); /* MARK: COMPARING */ diff --git a/modules/pico_icmp6.c b/modules/pico_icmp6.c index 40bbc727..1c941f06 100644 --- a/modules/pico_icmp6.c +++ b/modules/pico_icmp6.c @@ -933,6 +933,9 @@ static void pico_icmp6_ping_recv_reply(struct pico_frame *f) struct pico_icmp6_hdr *hdr = NULL; hdr = (struct pico_icmp6_hdr *)f->transport_hdr; + if ((uint32_t)f->transport_len < (uint32_t)PICO_ICMP6HDR_ECHO_REQUEST_SIZE) { + return; + } test.id = short_be(hdr->msg.info.echo_reply.id); test.seq = short_be(hdr->msg.info.echo_reply.seq); cookie = pico_tree_findKey(&f->dev->stack->IPV6Pings, &test); @@ -1031,6 +1034,10 @@ static void pico_icmp6_update_pmtu(struct pico_stack *S, struct pico_frame *f) const struct pico_ipv6_hdr *icmp_payload = NULL; struct pico_ipv6_path_id path_id; + /* The embedded IPv6 header must fit in the received frame. */ + if ((uint32_t)f->transport_len < (uint32_t)(PICO_ICMP6HDR_PKT_TOO_BIG_SIZE + PICO_SIZE_IP6HDR)) { + return; + } f->net_hdr = f->transport_hdr + PICO_ICMP6HDR_PKT_TOO_BIG_SIZE; icmp_payload = (struct pico_ipv6_hdr *)f->net_hdr; path_id.dst = icmp_payload->dst; diff --git a/modules/pico_igmp.c b/modules/pico_igmp.c index ce3f101d..03e086f7 100644 --- a/modules/pico_igmp.c +++ b/modules/pico_igmp.c @@ -428,11 +428,19 @@ static void pico_igmp_v2querier_expired(struct igmp_timer *t) static int pico_igmp_is_checksum_valid(struct pico_frame *f) { struct pico_ipv4_hdr *hdr = NULL; - uint8_t ihl = 24, datalen = 0; + uint16_t ihl = 20, datalen = 0; + uint32_t avail; hdr = (struct pico_ipv4_hdr *)f->net_hdr; - ihl = (uint8_t)((hdr->vhl & 0x0F) * 4); /* IHL is in 32bit words */ - datalen = (uint8_t)(short_be(hdr->len) - ihl); + ihl = (uint16_t)((hdr->vhl & 0x0F) * 4); /* IHL is in 32bit words */ + datalen = (uint16_t)(short_be(hdr->len) - ihl); + + /* The claimed payload must fit in the received frame. */ + if ((uint32_t)(f->transport_hdr - f->buffer) >= f->len) + return 0; + avail = (uint32_t)(f->len - (uint32_t)(f->transport_hdr - f->buffer)); + if (datalen > avail) + return 0; if (short_be(pico_checksum(f->transport_hdr, datalen)) == 0) return 1; diff --git a/modules/pico_ipv6.c b/modules/pico_ipv6.c index 8a4c9caa..b4347356 100644 --- a/modules/pico_ipv6.c +++ b/modules/pico_ipv6.c @@ -554,7 +554,7 @@ static int pico_ipv6_process_hopbyhop(struct pico_ipv6_exthdr *hbh, struct pico_ case PICO_IPV6_EXTHDR_OPT_PADN: optlen = (uint8_t)((*(option + 1)) + 2); /* plus type and len byte */ - if (optlen == 0) + if ((optlen > len) || (optlen == 0)) return -1; option += optlen; len = (uint8_t)(len - optlen); @@ -564,7 +564,7 @@ static int pico_ipv6_process_hopbyhop(struct pico_ipv6_exthdr *hbh, struct pico_ /* MLD package */ if (*(option + 1) == 2) must_align = 0; - if (optlen == 0) + if ((optlen > len) || (optlen == 0)) return -1; option += optlen; len = (uint8_t)(len - optlen); @@ -572,6 +572,8 @@ static int pico_ipv6_process_hopbyhop(struct pico_ipv6_exthdr *hbh, struct pico_ default: /* unknown option */ optlen = (uint8_t)(*(option + 1) + 2); /* plus type and len byte */ + if ((optlen > len) || (optlen == 0)) + return -1; switch ((*option) & PICO_IPV6_EXTHDR_OPT_ACTION_MASK) { case PICO_IPV6_EXTHDR_OPT_ACTION_SKIP: break; @@ -588,8 +590,6 @@ static int pico_ipv6_process_hopbyhop(struct pico_ipv6_exthdr *hbh, struct pico_ } ipv6_dbg("IPv6: option with type %u and length %u\n", *option, optlen); option += optlen; - if (optlen == 0) - return -1; len = (uint8_t)(len - optlen); } } @@ -677,9 +677,20 @@ static int pico_ipv6_check_headers_sequence(struct pico_frame *f) struct pico_ipv6_hdr *hdr = (struct pico_ipv6_hdr *)f->net_hdr; int ptr = sizeof(struct pico_ipv6_hdr); int cur_nexthdr = 6; /* Starts with nexthdr field in ipv6 pkt */ + int actual = (int)(f->len - (uint32_t)(f->net_hdr - f->buffer)); + int end = (int)sizeof(struct pico_ipv6_hdr) + (int)short_be(hdr->len); uint8_t nxthdr = hdr->nxthdr; + uint8_t optlen; + + /* A truncated packet may claim more payload than was actually received; + * never walk past the bytes we hold. */ + if (end > actual) + end = actual; + for (;;) { - uint8_t optlen = *(f->net_hdr + ptr + 1); + if (ptr + 2 > end) + return -1; /* ext header runs past the end of the packet */ + optlen = *(f->net_hdr + ptr + 1); if (optlen == 0) return 0; switch (nxthdr) { @@ -711,6 +722,8 @@ static int pico_ipv6_check_headers_sequence(struct pico_frame *f) nxthdr = *(f->net_hdr + ptr); if (optlen == 0) return -1; + if (ptr + (int)optlen > end) + return -1; /* ext header length exceeds the packet */ ptr += optlen; } } diff --git a/modules/pico_ipv6_nd.c b/modules/pico_ipv6_nd.c index 8882556b..290d0232 100644 --- a/modules/pico_ipv6_nd.c +++ b/modules/pico_ipv6_nd.c @@ -745,7 +745,7 @@ static int pico_nd_get_length_of_options(struct pico_frame *f, uint8_t **first_o return optlen; } -static int get_neigh_option(struct pico_frame *f, void *opt, uint8_t expected_opt) +static int get_neigh_option(struct pico_frame *f, void *opt, uint8_t expected_opt, size_t opt_size) { /* RFC 4861 * * Receivers MUST silently ignore any options they do not recognize @@ -757,25 +757,32 @@ static int get_neigh_option(struct pico_frame *f, void *opt, uint8_t expected_op int len = 0; uint8_t type = 0; int found = 0; + size_t copy_len = 0; optlen = pico_nd_get_length_of_options(f, &option); while (optlen > 0) { type = ((struct pico_icmp6_opt_na *)option)->type; len = ((struct pico_icmp6_opt_na *)option)->len; - optlen -= len << 3; /* len in units of 8 octets */ if (len <= 0) return -1; /* malformed option. */ + if ((size_t)(len << 3) > (size_t)optlen) + return -1; /* option extends past the end of the option area. */ + optlen -= len << 3; /* len in units of 8 octets */ if (type == expected_opt) { if (found > 0) return -1; /* malformed option: option is there twice. */ - if (expected_opt == PICO_ND_OPT_REDIRECT) { - memcpy(opt, option, sizeof(struct pico_icmp6_opt_redirect)); - } else { - memcpy(opt, option, (size_t)(len << 3)); - } + if (expected_opt == PICO_ND_OPT_REDIRECT) + copy_len = sizeof(struct pico_icmp6_opt_redirect); + else + copy_len = (size_t)(len << 3); + + if (copy_len > opt_size) + return -1; /* option larger than the destination buffer. */ + + memcpy(opt, option, copy_len); found++; } @@ -943,7 +950,7 @@ static void pico_ipv6_neighbor_from_unsolicited(struct pico_frame *f) 0 }; struct pico_ipv6_hdr *ip = (struct pico_ipv6_hdr *)f->net_hdr; - int valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC); + int valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC, sizeof(opt)); if (!pico_ipv6_is_unspecified(ip->src.addr)) { n = pico_get_neighbor_from_ncache(f->dev->stack, &ip->src); @@ -1114,7 +1121,7 @@ static int neigh_sol_validate_unspec(struct pico_frame *f) struct pico_icmp6_opt_lladdr opt = { 0 }; - int valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC); + int valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC, sizeof(opt)); if (!f->dev->mode && pico_ipv6_is_solnode_multicast(hdr->dst.addr, f->dev) == 0) { return -1; } @@ -1159,7 +1166,7 @@ static int neigh_sol_process(struct pico_frame *f) }; icmp6_hdr = (struct pico_icmp6_hdr *)f->transport_hdr; - valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC); + valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC, sizeof(opt)); if (valid_lladdr < 0) return -1; /* Malformed packet. */ @@ -1525,7 +1532,7 @@ static int router_sol_validity_checks(struct pico_frame *f) * source link-layer address option in the message. */ /* Check for SLLAO if the IP source address is UNSPECIFIED */ - sllao_present = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC); + sllao_present = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC, sizeof(opt)); if (pico_ipv6_is_unspecified(hdr->src.addr)) { /* Frame is not valid when SLLAO is present if IP6-SRC is UNSPEC. */ if (sllao_present) { @@ -1682,14 +1689,14 @@ static int radv_process(struct pico_frame *f) hdr = (struct pico_ipv6_hdr *)f->net_hdr; icmp6_hdr = (struct pico_icmp6_hdr *)f->transport_hdr; - optres_prefix = get_neigh_option(f, &prefix_option, PICO_ND_OPT_PREFIX); + optres_prefix = get_neigh_option(f, &prefix_option, PICO_ND_OPT_PREFIX, sizeof(prefix_option)); if (optres_prefix < 0) { /* Malformed packet */ return -1; } - sllao = get_neigh_option(f, &lladdr_src, PICO_ND_OPT_LLADDR_SRC); + sllao = get_neigh_option(f, &lladdr_src, PICO_ND_OPT_LLADDR_SRC, sizeof(lladdr_src)); if (sllao < 0) { /* Malformed packet */ @@ -1703,7 +1710,7 @@ static int radv_process(struct pico_frame *f) } #ifdef PICO_6LOWPAN_IPHC_ENABLED - context_option_valid = get_neigh_option(f, &co, PICO_ND_OPT_6CO); + context_option_valid = get_neigh_option(f, &co, PICO_ND_OPT_6CO, sizeof(co)); if (context_option_valid < 0) { /* Malformed packet */ return -1; @@ -1716,7 +1723,7 @@ static int radv_process(struct pico_frame *f) } #endif - abro_valid = get_neigh_option(f, &abro, PICO_ND_OPT_ABRO); + abro_valid = get_neigh_option(f, &abro, PICO_ND_OPT_ABRO, sizeof(abro)); if (abro_valid < 0) { /* Malformed packet */ return -1; @@ -1781,7 +1788,7 @@ static int radv_process(struct pico_frame *f) * router link has to be set before calling pico_ipv6_set_router_mtu */ struct pico_icmp6_opt_mtu mtu_option; - int mtu_valid = get_neigh_option(f, &mtu_option, PICO_ND_OPT_MTU); + int mtu_valid = get_neigh_option(f, &mtu_option, PICO_ND_OPT_MTU, sizeof(mtu_option)); if (mtu_valid > 0) { pico_ipv6_set_router_mtu(f->dev->stack, &hdr->src, long_be(mtu_option.mtu)); } @@ -1844,7 +1851,7 @@ static int neigh_adv_option_len_validity_check(struct pico_frame *f) }; int valid_lladdr = 0; - valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC); + valid_lladdr = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_SRC, sizeof(opt)); if (valid_lladdr < 0) return -1; @@ -1897,7 +1904,7 @@ static int neigh_adv_process(struct pico_frame *f) struct pico_icmp6_opt_lladdr opt = { 0 }; - int optres = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_TGT); + int optres = get_neigh_option(f, &opt, PICO_ND_OPT_LLADDR_TGT, sizeof(opt)); icmp6_hdr = (struct pico_icmp6_hdr *)f->transport_hdr; if (optres < 0) { /* Malformed packet: option field cannot be processed. */ @@ -2011,8 +2018,8 @@ static int redirect_process(struct pico_frame *f) redirect_hdr = &(icmp6_hdr->msg.info.redirect); /* Check the options */ - optres_lladdr = get_neigh_option(f, &opt_ll, PICO_ND_OPT_LLADDR_TGT); - optres_redirect = get_neigh_option(f, &opt_redirect, PICO_ND_OPT_REDIRECT); + optres_lladdr = get_neigh_option(f, &opt_ll, PICO_ND_OPT_LLADDR_TGT, sizeof(opt_ll)); + optres_redirect = get_neigh_option(f, &opt_redirect, PICO_ND_OPT_REDIRECT, sizeof(opt_redirect)); if (optres_lladdr < 0 || optres_redirect < 0) { /* Malformed packet */ diff --git a/modules/pico_mdns.c b/modules/pico_mdns.c index 93081996..02f7321e 100644 --- a/modules/pico_mdns.c +++ b/modules/pico_mdns.c @@ -1960,12 +1960,13 @@ pico_mdns_populate_antree(struct pico_stack *S, char *name, uint16_t qtype, uint * * @param question DNS question to parse and handle. * @param packet Received packet in which the DNS question was present. + * @param end One past the last valid byte of the packet * @return mDNS record tree with possible answer to the question. Can possibly * be empty. * ****************************************************************************/ static pico_mdns_rtree pico_mdns_handle_single_question(struct pico_dns_question *question, - pico_dns_packet *packet) + pico_dns_packet *packet, uint8_t *end) { struct pico_mdns_cookie *cookie = NULL; PICO_MDNS_RTREE_DECLARE(antree); @@ -1979,7 +1980,8 @@ pico_mdns_handle_single_question(struct pico_dns_question *question, } /* Decompress single DNS question */ - qname_original = pico_dns_question_decompress(question, packet); + qname_original = pico_dns_question_decompress(question, packet, + (size_t)(end - (uint8_t *)packet)); mdns_dbg("Question RCVD for '%s'\n", question->qname); if (NULL != question->qname) { @@ -2119,6 +2121,39 @@ pico_mdns_handle_single_additional(struct pico_stack *S, struct pico_mdns_record return 0; } +/* **************************************************************************** + * Walks a compressed DNS name in [name, end). + * + * @param name Start of the compressed name in the packet + * @param end One past the last valid byte of the packet + * @return Byte past the terminating zero or compression pointer, or NULL + * when the name is malformed or not fully contained in the range. + * ****************************************************************************/ +static uint8_t * +pico_mdns_name_end(uint8_t *name, uint8_t *end) +{ + uint8_t *label = name; + + while (label < end) { + uint8_t c = *label; + + if (c == 0x00) + return label + 1; + + if (c >= 0xC0) { + if (label + 1 >= end) + return NULL; + return label + 2; + } + + if (c > 63 || label + c + 1 > end) + return NULL; + + label += c + 1; + } + return NULL; +} + /* **************************************************************************** * Handles a flat chunk of memory as if it were all questions in it. * Generates a tree with responses if there are any questions for records for @@ -2128,13 +2163,15 @@ pico_mdns_handle_single_additional(struct pico_stack *S, struct pico_mdns_record * Will point to right after the question section on return. * @param qdcount Amount of questions contained in the packet * @param packet DNS packet where the questions are present. + * @param end One past the last valid byte of the packet * @return Tree with possible responses on the questions. * ****************************************************************************/ static pico_mdns_rtree -pico_mdns_handle_data_as_questions (struct pico_stack *S, - uint8_t **ptr, - uint16_t qdcount, - pico_dns_packet *packet) +pico_mdns_handle_data_as_questions(struct pico_stack *S, + uint8_t **ptr, + uint16_t qdcount, + pico_dns_packet *packet, + uint8_t *end) { PICO_MDNS_RTREE_DECLARE(antree); PICO_MDNS_RTREE_DECLARE(rtree); @@ -2148,17 +2185,25 @@ pico_mdns_handle_data_as_questions (struct pico_stack *S, } for (i = 0; i < qdcount; i++) { + uint8_t *name_end = pico_mdns_name_end(*ptr, end); + + /* Reject the packet when a question is not fully contained in it */ + if (!name_end || + name_end + sizeof(struct pico_dns_question_suffix) > end) { + pico_err = PICO_ERR_EINVAL; + return antree; + } + /* Set qname of the question to the correct location */ question.qname = (char *)(*ptr); /* Set qsuffix of the question to the correct location */ - question.qsuffix = (struct pico_dns_question_suffix *) - (question.qname + pico_dns_namelen_comp(question.qname) + 1); + question.qsuffix = (struct pico_dns_question_suffix *)name_end; question.stack = S; /* Handle a single question and merge the returned tree */ - rtree = pico_mdns_handle_single_question(&question, packet); + rtree = pico_mdns_handle_single_question(&question, packet, end); pico_tree_merge(&antree, &rtree); pico_tree_destroy(&rtree, NULL); @@ -2178,7 +2223,8 @@ pico_mdns_handle_data_as_answers_generic(struct pico_stack *S, uint8_t **ptr, uint16_t count, pico_dns_packet *packet, - uint8_t type) + uint8_t type, + uint8_t *end) { struct pico_mdns_record mdns_answer = { .record = NULL, .current_ttl = 0, @@ -2199,20 +2245,34 @@ pico_mdns_handle_data_as_answers_generic(struct pico_stack *S, /* (just like in pico_mdns_record_am_i_lexi_later) */ for (i = 0; i < count; i++) { + uint8_t *name_end = pico_mdns_name_end(*ptr, end); + + /* Reject the packet when a record is not fully contained in it */ + if (!name_end || + name_end + sizeof(struct pico_dns_record_suffix) > end) { + pico_err = PICO_ERR_EINVAL; + return -1; + } + /* Set rname of the record to the correct location */ answer.rname = (char *)(*ptr); /* Set rsuffix of the record to the correct location */ - answer.rsuffix = (struct pico_dns_record_suffix *) - (answer.rname + - pico_dns_namelen_comp(answer.rname) + 1u); + answer.rsuffix = (struct pico_dns_record_suffix *)name_end; /* Set rdata of the record to the correct location */ answer.rdata = (uint8_t *) answer.rsuffix + sizeof(struct pico_dns_record_suffix); + /* The rdata must be fully contained in the packet as well */ + if (end - answer.rdata < (ptrdiff_t)short_be(answer.rsuffix->rdlength)) { + pico_err = PICO_ERR_EINVAL; + return -1; + } + /* Make an mDNS record from the DNS answer */ - orname = pico_dns_record_decompress(&answer, packet); + orname = pico_dns_record_decompress(&answer, packet, + (size_t)(end - (uint8_t *)packet)); if (NULL != answer.rname) { mdns_answer.record = &answer; mdns_answer.record->rname_length = (uint16_t)(pico_dns_strlen(answer.rname) + 1u); @@ -2661,13 +2721,15 @@ pico_mdns_reply(struct pico_stack *S, pico_mdns_rtree *antree, struct pico_ip4 p * @param packet DNS packet in which to look for known answers * @param ancount Amount of answers in the DNS packet * @param data Answer section of the DNS packet as a flat chunk of memory. + * @param end One past the last valid byte of the packet * @return 0 K.A.S. could be properly applied, something else when not. * ****************************************************************************/ static int pico_mdns_apply_k_a_s(pico_mdns_rtree *rtree, pico_dns_packet *packet, uint16_t ancount, - uint8_t **data) + uint8_t **data, + uint8_t *end) { struct pico_tree_node *node = NULL, *next = NULL; struct pico_mdns_record *record = NULL, ka = { @@ -2685,18 +2747,33 @@ pico_mdns_apply_k_a_s(pico_mdns_rtree *rtree, } for (i = 0; i < ancount; i++) { + uint8_t *name_end = pico_mdns_name_end(*data, end); + + /* Reject the packet when a record is not fully contained in it */ + if (!name_end || + name_end + sizeof(struct pico_dns_record_suffix) > end) { + pico_err = PICO_ERR_EINVAL; + return -1; + } + /* Set rname of the record to the correct location */ answer.rname = (char *)(*data); /* Set rsuffix of the record to the correct location */ - answer.rsuffix = (struct pico_dns_record_suffix *) - (answer.rname + pico_dns_namelen_comp(answer.rname) + 1u); + answer.rsuffix = (struct pico_dns_record_suffix *)name_end; /* Set rdata of the record to the correct location */ answer.rdata = (uint8_t *) answer.rsuffix + sizeof(struct pico_dns_record_suffix); - pico_dns_record_decompress(&answer, packet); + /* The rdata must be fully contained in the packet as well */ + if (end - answer.rdata < (ptrdiff_t)short_be(answer.rsuffix->rdlength)) { + pico_err = PICO_ERR_EINVAL; + return -1; + } + + pico_dns_record_decompress(&answer, packet, + (size_t)(end - (uint8_t *)packet)); ka.record = &answer; /* If the answer is in the record vector */ @@ -2721,10 +2798,12 @@ pico_mdns_apply_k_a_s(pico_mdns_rtree *rtree, * * @param packet Received packet * @param peer IPv4 address of the peer who sent the received packet. + * @param end One past the last valid byte of the packet * @return Returns 0 when the query packet is properly handled. * ****************************************************************************/ static int -pico_mdns_handle_query_packet(struct pico_stack *S, pico_dns_packet *packet, struct pico_ip4 peer) +pico_mdns_handle_query_packet(struct pico_stack *S, pico_dns_packet *packet, + struct pico_ip4 peer, uint8_t *end) { PICO_MDNS_RTREE_DECLARE(antree); uint16_t qdcount = 0, ancount = 0; @@ -2735,7 +2814,7 @@ pico_mdns_handle_query_packet(struct pico_stack *S, pico_dns_packet *packet, str /* Generate a list of answers */ qdcount = short_be(packet->qdcount); - antree = pico_mdns_handle_data_as_questions(S, &data, qdcount, packet); + antree = pico_mdns_handle_data_as_questions(S, &data, qdcount, packet, end); if (pico_tree_count(&antree) == 0) { mdns_dbg("No records found that correspond with this query!\n"); return 0; @@ -2743,7 +2822,7 @@ pico_mdns_handle_query_packet(struct pico_stack *S, pico_dns_packet *packet, str /* Apply Known Answer Suppression */ ancount = short_be(packet->ancount); - if (pico_mdns_apply_k_a_s(&antree, packet, ancount, &data)) { + if (pico_mdns_apply_k_a_s(&antree, packet, ancount, &data, end)) { mdns_dbg("Could not apply known answer suppression!\n"); return -1; } @@ -2761,10 +2840,12 @@ pico_mdns_handle_query_packet(struct pico_stack *S, pico_dns_packet *packet, str * * @param packet Received probe packet. * @param peer IPv4 address of the peer who sent the probe packet. + * @param end One past the last valid byte of the packet * @return Returns 0 when the probe packet is properly handled. * ****************************************************************************/ static int -pico_mdns_handle_probe_packet(struct pico_stack *S, pico_dns_packet *packet, struct pico_ip4 peer) +pico_mdns_handle_probe_packet(struct pico_stack *S, pico_dns_packet *packet, + struct pico_ip4 peer, uint8_t *end) { PICO_MDNS_RTREE_DECLARE(antree); uint16_t qdcount = 0, nscount = 0; @@ -2775,11 +2856,11 @@ pico_mdns_handle_probe_packet(struct pico_stack *S, pico_dns_packet *packet, str /* Generate a list of answers */ qdcount = short_be(packet->qdcount); - antree = pico_mdns_handle_data_as_questions(S, &data, qdcount, packet); + antree = pico_mdns_handle_data_as_questions(S, &data, qdcount, packet, end); /* Check for Simultaneous Probe Tiebreaking */ nscount = short_be(packet->nscount); - pico_mdns_handle_data_as_answers_generic(S, &data, nscount, packet, 1); + pico_mdns_handle_data_as_answers_generic(S, &data, nscount, packet, 1, end); /* Try to reply with the answers */ if (pico_tree_count(&antree) != 0) { @@ -2795,10 +2876,12 @@ pico_mdns_handle_probe_packet(struct pico_stack *S, pico_dns_packet *packet, str * Handles a single incoming answer packet. * * @param packet Received answer packet. + * @param end One past the last valid byte of the packet * @return Returns 0 when the response packet is properly handled. * ****************************************************************************/ static int -pico_mdns_handle_response_packet(struct pico_stack *S, pico_dns_packet *packet) +pico_mdns_handle_response_packet(struct pico_stack *S, pico_dns_packet *packet, + uint8_t *end) { uint8_t *data = NULL; uint16_t ancount = 0; @@ -2808,7 +2891,7 @@ pico_mdns_handle_response_packet(struct pico_stack *S, pico_dns_packet *packet) /* Generate a list of answers */ ancount = short_be(packet->ancount); - if (pico_mdns_handle_data_as_answers_generic(S, &data, ancount, packet, 0)) { + if (pico_mdns_handle_data_as_answers_generic(S, &data, ancount, packet, 0, end)) { mdns_dbg("Could not handle data as answers\n"); return -1; } @@ -2828,11 +2911,23 @@ pico_mdns_handle_response_packet(struct pico_stack *S, pico_dns_packet *packet) static int pico_mdns_recv(struct pico_stack *S, void *buf, int buflen, struct pico_ip4 peer) { - pico_dns_packet *packet = (pico_dns_packet *) buf; - uint16_t qdcount = short_be(packet->qdcount); - uint16_t ancount = short_be(packet->ancount); - uint16_t authcount = short_be(packet->nscount); - uint16_t addcount = short_be(packet->arcount); + pico_dns_packet *packet = NULL; + uint8_t *end = NULL; + uint16_t qdcount = 0, ancount = 0; + uint16_t authcount = 0, addcount = 0; + + /* The DNS header must be fully present before any field is read */ + if (!buf || buflen < (int)sizeof(struct pico_dns_header)) { + pico_err = PICO_ERR_EINVAL; + return -1; + } + + packet = (pico_dns_packet *)buf; + end = (uint8_t *)buf + buflen; + qdcount = short_be(packet->qdcount); + ancount = short_be(packet->ancount); + authcount = short_be(packet->nscount); + addcount = short_be(packet->arcount); /* RFC6762: */ /* 18.3: Messages received with an opcode other than zero MUST be silently */ @@ -2843,7 +2938,6 @@ pico_mdns_recv(struct pico_stack *S, void *buf, int buflen, struct pico_ip4 peer mdns_dbg(">>>>>>> QDcount: %u, ANcount: %u, NScount: %u, ARcount: %u\n", qdcount, ancount, authcount, addcount); - IGNORE_PARAMETER(buflen); IGNORE_PARAMETER(addcount); /* DNS PACKET TYPE DETERMINATION */ @@ -2851,14 +2945,14 @@ pico_mdns_recv(struct pico_stack *S, void *buf, int buflen, struct pico_ip4 peer if (authcount > 0) { mdns_dbg(">>>>>>> RCVD a mDNS probe query:\n"); /* Packet is probe query */ - if (pico_mdns_handle_probe_packet(S, packet, peer) < 0) { + if (pico_mdns_handle_probe_packet(S, packet, peer, end) < 0) { mdns_dbg("Could not handle mDNS probe query!\n"); return -1; } } else { mdns_dbg(">>>>>>> RCVD a plain mDNS query:\n"); /* Packet is a plain query */ - if (pico_mdns_handle_query_packet(S, packet, peer) < 0) { + if (pico_mdns_handle_query_packet(S, packet, peer, end) < 0) { mdns_dbg("Could not handle plain DNS query!\n"); return -1; } @@ -2867,7 +2961,7 @@ pico_mdns_recv(struct pico_stack *S, void *buf, int buflen, struct pico_ip4 peer if (ancount > 0) { mdns_dbg(">>>>>>> RCVD a mDNS response:\n"); /* Packet is a response */ - if (pico_mdns_handle_response_packet(S, packet) < 0) { + if (pico_mdns_handle_response_packet(S, packet, end) < 0) { mdns_dbg("Could not handle DNS response!\n"); return -1; } diff --git a/modules/pico_nat.c b/modules/pico_nat.c index dc07d105..a0a9529d 100644 --- a/modules/pico_nat.c +++ b/modules/pico_nat.c @@ -518,6 +518,8 @@ int pico_ipv4_nat_outbound(struct pico_stack *S, struct pico_frame *f, struct pi tuple = pico_ipv4_nat_find_tuple(S, 0, &net->src, trans->sport, net->proto); if (!tuple) tuple = pico_ipv4_nat_generate_tuple(S, f); + if (!tuple) + return -1; /* replace src IP and src PORT */ net->src = tuple->nat_addr; @@ -536,6 +538,8 @@ int pico_ipv4_nat_outbound(struct pico_stack *S, struct pico_frame *f, struct pi tuple = pico_ipv4_nat_find_tuple(S, 0, &net->src, trans->sport, net->proto); if (!tuple) tuple = pico_ipv4_nat_generate_tuple(S, f); + if (!tuple) + return -1; /* replace src IP and src PORT */ net->src = tuple->nat_addr; diff --git a/modules/pico_olsr.c b/modules/pico_olsr.c index 778c9c1f..bd9a6f8a 100644 --- a/modules/pico_olsr.c +++ b/modules/pico_olsr.c @@ -895,6 +895,13 @@ static void olsr_recv(struct pico_stack *S, uint8_t *buffer, uint32_t len) struct olsr_route_entry *origin; msg = (struct olsrmsg *) (buffer + parsed); + /* Trust msg->size only if it covers the fixed header and stays + * within the packet; otherwise stop parsing. */ + if ((uint32_t)short_be(msg->size) < sizeof(struct olsrmsg) || + (uint32_t)short_be(msg->size) > (len - parsed)) { + break; + } + if (pico_ipv4_link_find(S, &msg->orig) != NULL) { parsed += short_be(msg->size); continue; diff --git a/modules/pico_sntp_client.c b/modules/pico_sntp_client.c index 41bff199..38295202 100644 --- a/modules/pico_sntp_client.c +++ b/modules/pico_sntp_client.c @@ -86,6 +86,7 @@ struct sntp_server_ns_cookie pico_time stamp; /* Timestamp of the moment the sntp packet is sent */ char *hostname; /* Hostname of the (s)ntp server*/ struct pico_socket *sock; /* Socket which contains the cookie */ + struct pico_stack *stack; /* Stack the cookie belongs to (set before sock) */ void (*cb_synced)(pico_err_t status); /* Callback function for telling the user whether/when the time is synchronised */ uint32_t timer; /* Timer that will signal timeout */ @@ -123,7 +124,7 @@ static void pico_sntp_cleanup(struct sntp_server_ns_cookie *ck, pico_err_t statu if (!ck) return; - pico_timer_cancel(ck->sock->stack, ck->timer); + pico_timer_cancel(ck->stack, ck->timer); ck->cb_synced(status); if (ck->sock) @@ -318,7 +319,7 @@ static void dnsCallback(char *ip, void *arg) #endif if (retval >= 0) { - retval = pico_sntp_sync_start(ck->sock->stack, ck, &address); + retval = pico_sntp_sync_start(ck->stack, ck, &address); if (retval < 0) pico_sntp_cleanup(ck, PICO_ERR_ENOTCONN); } @@ -330,7 +331,7 @@ static void dnsCallback(char *ip, void *arg) static int pico_sntp_sync_start_dns_ipv4(struct pico_stack *S, const char *sntp_server, void (*cb_synced)(pico_err_t status)) { struct sntp_server_ns_cookie *ck; - (void)S; + int retval = -1; if (sntp_server == NULL) { pico_err = PICO_ERR_EINVAL; return -1; @@ -347,6 +348,7 @@ static int pico_sntp_sync_start_dns_ipv4(struct pico_stack *S, const char *sntp_ ck->stamp = 0ull; ck->rec = 0; ck->sock = NULL; + ck->stack = S; ck->hostname = PICO_ZALLOC(strlen(sntp_server) + 1); if (!ck->hostname) { PICO_FREE(ck); @@ -363,6 +365,15 @@ static int pico_sntp_sync_start_dns_ipv4(struct pico_stack *S, const char *sntp_ return -1; } + ck->cb_synced = cb_synced; + sntp_dbg("Resolving A %s\n", ck->hostname); + retval = pico_dns_client_getaddr(S, sntp_server, &dnsCallback, ck); + if (retval != 0) { + PICO_FREE(ck->hostname); + PICO_FREE(ck); + return -1; + } + return 0; } #endif @@ -380,6 +391,7 @@ static int pico_sntp_sync_start_ipv4(struct pico_stack *S, union pico_address *a ck->stamp = 0ull; ck->rec = 0; ck->sock = NULL; + ck->stack = S; /* Set the given IP address as hostname, allocate the maximum IPv4 string length + 1 */ ck->hostname = PICO_ZALLOC(15 + 1); if (!ck->hostname) { @@ -422,6 +434,7 @@ static int pico_sntp_sync_start_dns_ipv6(struct pico_stack *S, const char *sntp_ } ck6->proto = PICO_PROTO_IPV6; + ck6->stack = S; ck6->hostname = PICO_ZALLOC(strlen(sntp_server) + 1); if (!ck6->hostname) { PICO_FREE(ck6); @@ -460,6 +473,7 @@ static int pico_sntp_sync_start_ipv6(struct pico_stack *S, union pico_address *a ck6->stamp = 0ull; ck6->rec = 0; ck6->sock = NULL; + ck6->stack = S; ck6->cb_synced = cb_synced; /* Set the given IP address as hostname, allocate the maximum IPv6 string length + 1 */ ck6->hostname = PICO_ZALLOC(39 + 1); diff --git a/modules/pico_tcp.c b/modules/pico_tcp.c index b30085d9..38b2d98a 100644 --- a/modules/pico_tcp.c +++ b/modules/pico_tcp.c @@ -924,9 +924,19 @@ static int tcp_parse_options(struct pico_frame *f) break; case PICO_TCP_OPTION_SACK: - tcp_rcv_sack(t, opt + i, len - 2); + { + /* The declared SACK length is independent of the options + * region, so cap the read to the bytes actually left. */ + int remaining = (int)f->transport_len - (int)PICO_SIZE_TCPHDR - (int)i; + int sack_len = (int)len - 2; + if (remaining < 0) + remaining = 0; + if (sack_len > remaining) + sack_len = remaining; + tcp_rcv_sack(t, opt + i, sack_len); i = i + len - 2; break; + } default: tcp_dbg_options("TCP: received unsupported option %u\n", type); i = i + len - 2; diff --git a/test/unit/modunit_pico_dns_common.c b/test/unit/modunit_pico_dns_common.c index 3a595dee..f4550c2a 100644 --- a/test/unit/modunit_pico_dns_common.c +++ b/test/unit/modunit_pico_dns_common.c @@ -1039,15 +1039,39 @@ START_TEST(tc_pico_dns_namelen_comp) /* MARK: dns_namelen_comp */ END_TEST START_TEST(tc_pico_dns_decompress_name) /* MARK: dns_decompress_name */ { - char name[] = "\4mail\xc0\x02"; - char name2[] = "\xc0\x02"; - char buf[] = "00\6google\3com"; + /* Single packet buffers: the compressed name always lies inside the + * packet, as in real mDNS/DNS-client usage. */ + unsigned char buf1[] = { + 0x04, 'm', 'a', 'i', 'l', 0xC0, 0x08, 0x00, + 0x06, 'g', 'o', 'o', 'g', 'l', 'e', + 0x03, 'c', 'o', 'm', 0x00 + }; + unsigned char buf2[] = { + 0xC0, 0x02, + 0x06, 'g', 'o', 'o', 'g', 'l', 'e', + 0x03, 'c', 'o', 'm', 0x00 + }; + unsigned char buf3[] = { + 0x03, 'f', 'o', 'o', 0x03, 'b', 'a', 'r', 0x00 + }; + unsigned char buf4[] = { + 0x03, 'f', 'o', 'o', 0xC0, 0x3F + }; + unsigned char buf5[] = { + 0x03, 'f', 'o', 'o' + }; + unsigned char buf6[] = { + 0x20, 'a' + }; + unsigned char buf7[] = { + 0x03, 'f', 'o', 'o', 0xC0, 0x04, 0xC0, 0x05, 0x00 + }; char *ret; printf("*********************** starting %s * \n", __func__); /* Test normal DNS name compression */ - ret = pico_dns_decompress_name(name, (pico_dns_packet *)buf); + ret = pico_dns_decompress_name((char *)buf1, (pico_dns_packet *)buf1, sizeof(buf1)); /* Fail conditions */ fail_unless(ret != NULL, "Name ptr returned is NULL"); @@ -1058,7 +1082,7 @@ START_TEST(tc_pico_dns_decompress_name) /* MARK: dns_decompress_name */ ret = NULL; /* Test when there is only a pointer */ - ret = pico_dns_decompress_name(name2, (pico_dns_packet *)buf); + ret = pico_dns_decompress_name((char *)buf2, (pico_dns_packet *)buf2, sizeof(buf2)); /* Fail conditions */ fail_unless(ret != NULL, "Name ptr returned is NULL"); @@ -1067,6 +1091,34 @@ START_TEST(tc_pico_dns_decompress_name) /* MARK: dns_decompress_name */ /* Free memory */ PICO_FREE(ret); ret = NULL; + + /* Test a plain name without compression */ + ret = pico_dns_decompress_name((char *)buf3, (pico_dns_packet *)buf3, sizeof(buf3)); + + /* Fail conditions */ + fail_unless(ret != NULL, "Name ptr returned is NULL"); + fail_unless(strcmp(ret, "\3foo\3bar") == 0, "Not correctly decompressed: '%s'!\n", ret); + + /* Free memory */ + PICO_FREE(ret); + ret = NULL; + + /* Regression: compression pointer past the packet end must be rejected */ + ret = pico_dns_decompress_name((char *)buf4, (pico_dns_packet *)buf4, sizeof(buf4)); + fail_unless(ret == NULL, "OOB compression pointer accepted!\n"); + + /* Regression: name without terminator running off the packet end */ + ret = pico_dns_decompress_name((char *)buf5, (pico_dns_packet *)buf5, sizeof(buf5)); + fail_unless(ret == NULL, "Unterminated name past packet end accepted!\n"); + + /* Regression: label length running past the packet end */ + ret = pico_dns_decompress_name((char *)buf6, (pico_dns_packet *)buf6, sizeof(buf6)); + fail_unless(ret == NULL, "Label length past packet end accepted!\n"); + + /* Regression: a second compression pointer is not valid */ + ret = pico_dns_decompress_name((char *)buf7, (pico_dns_packet *)buf7, sizeof(buf7)); + fail_unless(ret == NULL, "Second compression pointer accepted!\n"); + printf("*********************** ending %s * \n", __func__); } END_TEST diff --git a/test/unit/modunit_pico_ipv6_nd.c b/test/unit/modunit_pico_ipv6_nd.c index ae2b0f4a..3bf8f177 100644 --- a/test/unit/modunit_pico_ipv6_nd.c +++ b/test/unit/modunit_pico_ipv6_nd.c @@ -1484,9 +1484,9 @@ START_TEST(tc_get_neigh_option) f->net_len = sizeof(struct pico_ipv6_hdr); /* Get neigh options from our RA (which has LL addr option and prefix option) */ - fail_unless(get_neigh_option(f, &opt_lladdr, PICO_ND_OPT_LLADDR_SRC) == 1, "our RA should have a valid LL addr option"); - fail_unless(get_neigh_option(f, &opt_prefix, PICO_ND_OPT_PREFIX) == 1, "our RA should have a valid prefix option"); - fail_unless(get_neigh_option(f, &opt_redirect, PICO_ND_OPT_REDIRECT) == 0, "our RA doesn't have a redirect option"); + fail_unless(get_neigh_option(f, &opt_lladdr, PICO_ND_OPT_LLADDR_SRC, sizeof(opt_lladdr)) == 1, "our RA should have a valid LL addr option"); + fail_unless(get_neigh_option(f, &opt_prefix, PICO_ND_OPT_PREFIX, sizeof(opt_prefix)) == 1, "our RA should have a valid prefix option"); + fail_unless(get_neigh_option(f, &opt_redirect, PICO_ND_OPT_REDIRECT, sizeof(opt_redirect)) == 0, "our RA doesn't have a redirect option"); /* Check if ll addr is valid */ /* len is stored as a number of bytes */ @@ -1516,8 +1516,8 @@ START_TEST(tc_get_neigh_option) f->net_len = sizeof(struct pico_ipv6_hdr); /* Get neigh options from our RA (which has LL addr option and prefix option, one option has bad length field) */ - fail_unless(get_neigh_option(f, &opt_lladdr, PICO_ND_OPT_LLADDR_SRC) < 0, "our RA has a bad len field, should have returned failure"); - fail_unless(get_neigh_option(f, &opt_prefix, PICO_ND_OPT_PREFIX) < 0, "our RA has a bad len field, should have returned failure"); + fail_unless(get_neigh_option(f, &opt_lladdr, PICO_ND_OPT_LLADDR_SRC, sizeof(opt_lladdr)) < 0, "our RA has a bad len field, should have returned failure"); + fail_unless(get_neigh_option(f, &opt_prefix, PICO_ND_OPT_PREFIX, sizeof(opt_prefix)) < 0, "our RA has a bad len field, should have returned failure"); /* Cleanup */ pico_frame_discard(f); @@ -1528,8 +1528,8 @@ START_TEST(tc_get_neigh_option) f->net_len = sizeof(struct pico_ipv6_hdr); /* Get neigh options from our RA (which has 2 * LL addr option and prefix option) */ - fail_unless(get_neigh_option(f, &opt_prefix, PICO_ND_OPT_PREFIX) == 1, "our RA has a double option, but not the prefix option. So retrieving this should have returned success"); - fail_unless(get_neigh_option(f, &opt_lladdr, PICO_ND_OPT_LLADDR_SRC) < 0, "our RA has a double option, should have returned failure"); + fail_unless(get_neigh_option(f, &opt_prefix, PICO_ND_OPT_PREFIX, sizeof(opt_prefix)) == 1, "our RA has a double option, but not the prefix option. So retrieving this should have returned success"); + fail_unless(get_neigh_option(f, &opt_lladdr, PICO_ND_OPT_LLADDR_SRC, sizeof(opt_lladdr)) < 0, "our RA has a double option, should have returned failure"); pico_frame_discard(f); pico_device_destroy(dummy_device); diff --git a/test/unit/modunit_pico_ipv6_pmtu.c b/test/unit/modunit_pico_ipv6_pmtu.c index b03df445..9ac5d9e5 100644 --- a/test/unit/modunit_pico_ipv6_pmtu.c +++ b/test/unit/modunit_pico_ipv6_pmtu.c @@ -46,6 +46,7 @@ START_TEST(pico_ipv6_pkt_too_big) f = pico_frame_alloc(sizeof(pkt1)); memcpy(f->buffer, pkt1, sizeof(pkt1)); f->transport_hdr = f->buffer + PICO_SIZE_ETHHDR + PICO_SIZE_IP6HDR; + f->transport_len = (uint16_t)(sizeof(pkt1) - PICO_SIZE_ETHHDR - PICO_SIZE_IP6HDR); memcpy(path_id.dst.addr, pkt1 + dst_offset, sizeof(path_id.dst.addr)); fail_if(pico_ipv6_path_add(S, &path_id, default_mtu) != PICO_PMTU_OK); pico_icmp6_process_in(S, NULL, f); @@ -429,6 +430,7 @@ START_TEST(pico_ipv6_pkt_too_big_ignorecode) f_mtu->dev = dummy_device->dev; memcpy(f_mtu->buffer, pkt17, sizeof(pkt17)); f_mtu->transport_hdr = f_mtu->buffer + PICO_SIZE_ETHHDR + PICO_SIZE_IP6HDR; + f_mtu->transport_len = (uint16_t)(sizeof(pkt17) - PICO_SIZE_ETHHDR - PICO_SIZE_IP6HDR); memcpy(path_id.dst.addr, pkt17 + dst_offset, sizeof(path_id.dst.addr)); fail_if(pico_ipv6_pmtu_get(S, &path_id) != default_mtu); pico_icmp6_process_in(S, NULL, f_mtu); diff --git a/test/unit/modunit_pico_mdns.c b/test/unit/modunit_pico_mdns.c index eb5f85d3..6c250021 100644 --- a/test/unit/modunit_pico_mdns.c +++ b/test/unit/modunit_pico_mdns.c @@ -1620,7 +1620,8 @@ START_TEST(tc_mdns_handle_data_as_questions) /* MARK: handle_data_as_questions * ptr = ((uint8_t *)packet + 12); - antree = pico_mdns_handle_data_as_questions(S, &ptr, 2, packet); + antree = pico_mdns_handle_data_as_questions(S, &ptr, 2, packet, + (uint8_t *)packet + len); fail_unless(2 == pico_tree_count(&antree), "pico_mdns_handle_data_as_questions returned error!\n"); @@ -1661,7 +1662,8 @@ START_TEST(tc_mdns_handle_data_as_answers) /* MARK: handle_data_as_answers */ ptr = ((uint8_t *)packet + 12); - ret = pico_mdns_handle_data_as_answers_generic(S, &ptr, 2, packet, 0); + ret = pico_mdns_handle_data_as_answers_generic(S, &ptr, 2, packet, 0, + (uint8_t *)packet + len); fail_unless(0 == ret, "mdns_handle_data_as_answers failed!\n"); printf("*********************** ending %s * \n", __func__); @@ -1701,12 +1703,96 @@ START_TEST(tc_mdns_handle_data_as_authorities) /* MARK: handle_data_as_authoriti ptr = ((uint8_t *)packet + 12); - ret = pico_mdns_handle_data_as_answers_generic(S, &ptr, 2, packet, 1); + ret = pico_mdns_handle_data_as_answers_generic(S, &ptr, 2, packet, 1, + (uint8_t *)packet + len); fail_unless(0 == ret, "mdns_handle_data_as_answers failed!\n"); printf("*********************** ending %s * \n", __func__); } END_TEST +START_TEST(tc_mdns_recv_truncated_packet) /* MARK: recv_truncated_packet */ +{ + pico_dns_packet *packet = NULL; + struct pico_ip4 peer = { + long_be(0x0A000001) + }; + uint8_t buf[64]; + int ret = 0; + struct pico_stack *S = NULL; + + printf("*********************** starting %s * \n", __func__); + + pico_stack_init(&S); + + /* Response with one answer whose name is truncated: no terminator */ + memset(buf, 0, sizeof(buf)); + packet = (pico_dns_packet *)buf; + packet->ancount = short_be(1); + buf[12] = 0x03; + buf[13] = 'f'; + buf[14] = 'o'; + buf[15] = 'o'; + + ret = pico_mdns_recv(S, buf, 16, peer); + fail_unless(-1 == ret, "truncated name accepted!\n"); + + /* Response with one answer whose rdlength runs past the packet end */ + memset(buf, 0, sizeof(buf)); + packet = (pico_dns_packet *)buf; + packet->ancount = short_be(1); + buf[12] = 0x03; + buf[13] = 'f'; + buf[14] = 'o'; + buf[15] = 'o'; + buf[16] = 0x00; + buf[17] = 0x00; + buf[18] = 0x01; + buf[19] = 0x80; + buf[20] = 0x01; + buf[21] = 0x00; + buf[22] = 0x00; + buf[23] = 0x00; + buf[24] = 0x78; + buf[25] = 0x01; + buf[26] = 0x00; + buf[27] = 0x0A; + buf[28] = 0x0A; + buf[29] = 0x00; + buf[30] = 0x01; + + ret = pico_mdns_recv(S, buf, 31, peer); + fail_unless(-1 == ret, "rdlength past packet end accepted!\n"); + + /* A fully contained answer is still accepted */ + memset(buf, 0, sizeof(buf)); + packet = (pico_dns_packet *)buf; + packet->ancount = short_be(1); + buf[12] = 0x03; + buf[13] = 'f'; + buf[14] = 'o'; + buf[15] = 'o'; + buf[16] = 0x00; + buf[17] = 0x00; + buf[18] = 0x01; + buf[19] = 0x80; + buf[20] = 0x01; + buf[21] = 0x00; + buf[22] = 0x00; + buf[23] = 0x00; + buf[24] = 0x78; + buf[25] = 0x00; + buf[26] = 0x04; + buf[27] = 0x0A; + buf[28] = 0x0A; + buf[29] = 0x00; + buf[30] = 0x01; + + ret = pico_mdns_recv(S, buf, 31, peer); + fail_unless(0 == ret, "valid response rejected!\n"); + + printf("*********************** ending %s * \n", __func__); +} +END_TEST START_TEST(tc_mdns_handle_data_as_additionals) /* MARK: handle_data_as_additionals */ { printf("*********************** starting %s * \n", __func__); @@ -1839,7 +1925,8 @@ START_TEST(tc_mdns_apply_known_answer_suppression) /* MARK: apply_k_a_s */ printf("Applying Known answer suppression...\n"); - ret = pico_mdns_apply_k_a_s(&rtree, packet, 1, &ptr); + ret = pico_mdns_apply_k_a_s(&rtree, packet, 1, &ptr, + (uint8_t *)packet + len); fail_unless(0 == ret, "mdns_apply_known_answer_suppression returned error!\n"); fail_unless(1 == pico_tree_count(&rtree), @@ -2188,6 +2275,7 @@ Suite *pico_suite(void) TCase *TCase_mdns_handle_data_as_questions = tcase_create("Unit test for mdns_handle_data_as_questions"); TCase *TCase_mdns_handle_data_as_answers = tcase_create("Unit test for mdns_handle_data_as_answers"); TCase *TCase_mdns_handle_data_as_authorities = tcase_create("Unit test for mdns_handle_data_as_authorities"); + TCase *TCase_mdns_recv_truncated_packet = tcase_create("Unit test for mdns_recv_truncated_packet"); TCase *TCase_mdns_handle_data_as_additionals = tcase_create("Unit test for mdns_handle_data_as_additionals"); /* Handling query packets */ @@ -2302,6 +2390,8 @@ Suite *pico_suite(void) suite_add_tcase(s, TCase_mdns_handle_data_as_answers); tcase_add_test(TCase_mdns_handle_data_as_authorities, tc_mdns_handle_data_as_authorities); suite_add_tcase(s, TCase_mdns_handle_data_as_authorities); + tcase_add_test(TCase_mdns_recv_truncated_packet, tc_mdns_recv_truncated_packet); + suite_add_tcase(s, TCase_mdns_recv_truncated_packet); tcase_add_test(TCase_mdns_handle_data_as_additionals, tc_mdns_handle_data_as_additionals); suite_add_tcase(s, TCase_mdns_handle_data_as_additionals); diff --git a/test/unit/modunit_pico_sntp_client.c b/test/unit/modunit_pico_sntp_client.c index 2ca8174c..c4d669ff 100644 --- a/test/unit/modunit_pico_sntp_client.c +++ b/test/unit/modunit_pico_sntp_client.c @@ -331,6 +331,7 @@ START_TEST(tc_dnsCallback) ck = PICO_ZALLOC(sizeof(struct sntp_server_ns_cookie)); ck->sock = sock; + ck->stack = S; dnsCallback(ip, ck); }