The pcapng file format uses a 16-bit length field in the option TLV (Type-Length-Value) encoding, limiting strings to UINT16_MAX bytes.
Add validation for string arguments to prevent silent truncation or buffer issues when callers pass excessively long strings. Signed-off-by: Stephen Hemminger <[email protected]> --- doc/guides/rel_notes/release_26_03.rst | 5 ++++ lib/pcapng/rte_pcapng.c | 37 ++++++++++++++++++++++---- lib/pcapng/rte_pcapng.h | 3 ++- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/doc/guides/rel_notes/release_26_03.rst b/doc/guides/rel_notes/release_26_03.rst index 39f381e561..eea01e44e9 100644 --- a/doc/guides/rel_notes/release_26_03.rst +++ b/doc/guides/rel_notes/release_26_03.rst @@ -148,6 +148,11 @@ API Changes * cfgfile: name must be less than CFG_NAME_LEN and value must be less than CFG_VALUE_LEN. +* **Updated the pcapng library.** + + * The length of comment strings is now validated. + Maximum allowable length is 2^16-1 because of pcapng file format. + ABI Changes ----------- diff --git a/lib/pcapng/rte_pcapng.c b/lib/pcapng/rte_pcapng.c index 5789c2cdca..914d6e7095 100644 --- a/lib/pcapng/rte_pcapng.c +++ b/lib/pcapng/rte_pcapng.c @@ -34,6 +34,9 @@ /* conversion from DPDK speed to PCAPNG */ #define PCAPNG_MBPS_SPEED 1000000ull +/* upper bound for strings in pcapng option data */ +#define PCAPNG_STR_MAX UINT16_MAX + /* upper bound for section, stats and interface blocks (in uint32_t) */ #define PCAPNG_BLKSIZ (2048 / sizeof(uint32_t)) @@ -218,9 +221,11 @@ rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, uint16_t link_type, char ifname_buf[IF_NAMESIZE]; char ifhw[256]; uint64_t speed = 0; + int ret; - if (rte_eth_dev_info_get(port, &dev_info) < 0) - return -1; + ret = rte_eth_dev_info_get(port, &dev_info); + if (ret < 0) + return -1; /* should be ret */ /* make something like an interface name */ if (ifname == NULL) { @@ -230,8 +235,14 @@ rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, uint16_t link_type, snprintf(ifname_buf, IF_NAMESIZE, "dpdk:%u", port); ifname = ifname_buf; } + } else if (strlen(ifname) > PCAPNG_STR_MAX) { + return -1; /* ENAMETOOLONG */ } + if ((ifdescr && strlen(ifdescr) > PCAPNG_STR_MAX) || + (filter && strlen(filter) > PCAPNG_STR_MAX)) + return -1; /* EINVAL */ + /* make a useful device hardware string */ dev = dev_info.device; if (dev) @@ -269,7 +280,7 @@ rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, uint16_t link_type, len += sizeof(uint32_t); if (len > sizeof(buf)) - return -1; + return -1; /* EINVAL */ hdr = (struct pcapng_interface_block *)buf; *hdr = (struct pcapng_interface_block) { @@ -336,6 +347,9 @@ rte_pcapng_write_stats(rte_pcapng_t *self, uint16_t port_id, RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); + if (comment && strlen(comment) > PCAPNG_STR_MAX) + return -EINVAL; + optlen = 0; if (ifrecv != UINT64_MAX) @@ -487,6 +501,10 @@ rte_pcapng_copy(uint16_t port_id, uint32_t queue, bool rss_hash; #ifdef RTE_LIBRTE_ETHDEV_DEBUG + /* + * Since this function is used in the fast path for packet capture + * skip argument validation checks unless debug is enabled. + */ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, NULL); #endif orig_len = rte_pktmbuf_pkt_len(md); @@ -535,8 +553,9 @@ rte_pcapng_copy(uint16_t port_id, uint32_t queue, if (rss_hash) optlen += pcapng_optlen(sizeof(uint8_t) + sizeof(uint32_t)); + /* Comment is silently truncated if necessary */ if (comment) - optlen += pcapng_optlen(strlen(comment)); + optlen += pcapng_optlen(strnlen(comment, PCAPNG_STR_MAX)); /* reserve trailing options and block length */ opt = (struct pcapng_option *) @@ -577,7 +596,7 @@ rte_pcapng_copy(uint16_t port_id, uint32_t queue, if (comment) opt = pcapng_add_option(opt, PCAPNG_OPT_COMMENT, comment, - strlen(comment)); + strnlen(comment, PCAPNG_STR_MAX)); /* Note: END_OPT necessary here. Wireshark doesn't do it. */ @@ -692,6 +711,14 @@ rte_pcapng_fdopen(int fd, struct timespec ts; uint64_t cycles; + if ((osname && strlen(osname) > PCAPNG_STR_MAX) || + (hardware && strlen(hardware) > PCAPNG_STR_MAX) || + (appname && strlen(appname) > PCAPNG_STR_MAX) || + (comment && strlen(comment) > PCAPNG_STR_MAX)) { + rte_errno = EINVAL; + return NULL; + } + self = malloc(sizeof(*self)); if (!self) { rte_errno = ENOMEM; diff --git a/lib/pcapng/rte_pcapng.h b/lib/pcapng/rte_pcapng.h index 8d0974c3fd..68e13c67e4 100644 --- a/lib/pcapng/rte_pcapng.h +++ b/lib/pcapng/rte_pcapng.h @@ -127,7 +127,8 @@ enum rte_pcapng_direction { * @param direction * The direction of the packer: receive, transmit or unknown. * @param comment - * Packet comment. + * Optional per packet comment. + * Truncated to UINT16_MAX characters. * * @return * - The pointer to the new mbuf formatted for pcapng_write -- 2.51.0
