From 31d33de0fb9cedcea8a303bb17f10ceab9c408f1 Mon Sep 17 00:00:00 2001 From: Geoff Martin Date: Fri, 12 Dec 2025 17:26:30 +0000 Subject: [PATCH 1/2] Make _z_iosli_copy_bytes() copy rather data than append and harden _z_iosli_copy(). --- include/zenoh-pico/protocol/iobuf.h | 5 +- src/protocol/iobuf.c | 5 ++ tests/z_iobuf_test.c | 92 +++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/include/zenoh-pico/protocol/iobuf.h b/include/zenoh-pico/protocol/iobuf.h index a768353cb..0d2c73677 100644 --- a/include/zenoh-pico/protocol/iobuf.h +++ b/include/zenoh-pico/protocol/iobuf.h @@ -82,8 +82,9 @@ static inline void _z_iosli_read_bytes(_z_iosli_t *ios, uint8_t *dst, size_t off static inline void _z_iosli_copy_bytes(_z_iosli_t *dst, const _z_iosli_t *src) { size_t length = _z_iosli_readable(src); assert(dst->_capacity >= length); - (void)memcpy(dst->_buf + dst->_w_pos, src->_buf + src->_r_pos, length); - dst->_w_pos += length; + (void)memcpy(dst->_buf, src->_buf + src->_r_pos, length); + dst->_r_pos = 0; + dst->_w_pos = length; } static inline void _z_iosli_write_bytes(_z_iosli_t *ios, const uint8_t *bs, size_t offset, size_t length) { assert(_z_iosli_writable(ios) >= length); diff --git a/src/protocol/iobuf.c b/src/protocol/iobuf.c index 96799ae39..518d846e0 100644 --- a/src/protocol/iobuf.c +++ b/src/protocol/iobuf.c @@ -102,6 +102,11 @@ void _z_iosli_copy(_z_iosli_t *dst, const _z_iosli_t *src) { dst->_buf = (uint8_t *)z_malloc(src->_capacity); if (dst->_buf != NULL) { (void)memcpy(dst->_buf, src->_buf, src->_capacity); + } else { + dst->_r_pos = 0; + dst->_w_pos = 0; + dst->_capacity = 0; + dst->_is_alloc = false; } } else { dst->_buf = src->_buf; diff --git a/tests/z_iobuf_test.c b/tests/z_iobuf_test.c index 2b34e14a3..833dfdacb 100644 --- a/tests/z_iobuf_test.c +++ b/tests/z_iobuf_test.c @@ -609,6 +609,95 @@ void test_wbuf_wrap_bytes(void) { printf("Ok\n"); } +static void fill_pattern(uint8_t *buf, size_t n) { + for (size_t i = 0; i < n; i++) { + buf[i] = (uint8_t)(0xA0u + (uint8_t)i); + } +} + +void test_copy_bytes_readable_region(void) { + _z_iosli_t src = _z_iosli_make(16); + _z_iosli_t dst = _z_iosli_make(16); + assert(src._buf); + assert(dst._buf); + + fill_pattern(src._buf, src._capacity); + + // Make readable window be [r_pos..w_pos) = [5..12) => length 7 + src._r_pos = 5; + src._w_pos = 12; + + // Dirty dst positions to ensure function overwrites them + dst._r_pos = 3; + dst._w_pos = 9; + memset(dst._buf, 0x00, dst._capacity); + + _z_iosli_copy_bytes(&dst, &src); + + // dst now contains exactly the readable bytes starting at offset 0 + assert(dst._r_pos == 0); + assert(dst._w_pos == 7); + assert(memcmp(dst._buf, src._buf + src._r_pos, 7) == 0); + + _z_iosli_clear(&src); + _z_iosli_clear(&dst); +} + +void test_copy_bytes_exact_capacity_fit(void) { + _z_iosli_t src = _z_iosli_make(32); + assert(src._buf); + + // Create a readable region length = 10 + src._r_pos = 2; + src._w_pos = 12; // len = 10 + fill_pattern(src._buf, src._capacity); + + _z_iosli_t dst = _z_iosli_make(10); + assert(dst._buf); + memset(dst._buf, 0xEE, dst._capacity); + + // Should succeed because dst capacity == readable length + _z_iosli_copy_bytes(&dst, &src); + + assert(dst._r_pos == 0); + assert(dst._w_pos == 10); + assert(memcmp(dst._buf, src._buf + src._r_pos, 10) == 0); + + _z_iosli_clear(&src); + _z_iosli_clear(&dst); +} + +void test_copy_bytes_does_not_alias(void) { + _z_iosli_t src = _z_iosli_make(16); + _z_iosli_t dst = _z_iosli_make(16); + assert(src._buf && dst._buf); + + fill_pattern(src._buf, src._capacity); + src._r_pos = 4; + src._w_pos = 9; // len = 5 + + _z_iosli_copy_bytes(&dst, &src); + + assert(dst._r_pos == 0); + assert(dst._w_pos == 5); + assert(memcmp(dst._buf, src._buf + src._r_pos, 5) == 0); + + // Mutate src readable region; dst should remain unchanged (deep copy) + src._buf[src._r_pos + 0] ^= 0xFF; + src._buf[src._r_pos + 1] ^= 0xFF; + + // Should differ + assert(memcmp(dst._buf, src._buf + src._r_pos, 5) != 0); + + // But dst still equals original pattern: + uint8_t expected_full[16]; + fill_pattern(expected_full, sizeof(expected_full)); + assert(memcmp(dst._buf, expected_full + 4, 5) == 0); + + _z_iosli_clear(&src); + _z_iosli_clear(&dst); +} + /*=============================*/ /* Main */ /*=============================*/ @@ -634,4 +723,7 @@ int main(void) { wbuf_reusable_write_zbuf_read(); } test_wbuf_wrap_bytes(); + test_copy_bytes_readable_region(); + test_copy_bytes_exact_capacity_fit(); + test_copy_bytes_does_not_alias(); } From eccea31cbedd066d76db303ddc3e322b15ab5920 Mon Sep 17 00:00:00 2001 From: Geoff Martin Date: Fri, 12 Dec 2025 18:29:49 +0000 Subject: [PATCH 2/2] Suppress memcpy flaw as checked above. --- include/zenoh-pico/protocol/iobuf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/zenoh-pico/protocol/iobuf.h b/include/zenoh-pico/protocol/iobuf.h index 0d2c73677..359ec98a9 100644 --- a/include/zenoh-pico/protocol/iobuf.h +++ b/include/zenoh-pico/protocol/iobuf.h @@ -82,6 +82,8 @@ static inline void _z_iosli_read_bytes(_z_iosli_t *ios, uint8_t *dst, size_t off static inline void _z_iosli_copy_bytes(_z_iosli_t *dst, const _z_iosli_t *src) { size_t length = _z_iosli_readable(src); assert(dst->_capacity >= length); + // SAFETY: Checked by assert above. + // Flawfinder: ignore [CWE-120] (void)memcpy(dst->_buf, src->_buf + src->_r_pos, length); dst->_r_pos = 0; dst->_w_pos = length;