From 819c61265d9e3c74516c45e88c4645b4d055bb74 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 25 Sep 2026 10:22:39 +0200 Subject: [PATCH] src: introduce memmove wrapper using memmove_s if available Ticket: 6028 memmove_s is a C11 safer version adding a fourth argument which is the total size of the destination buffer, so as to check there is no risk of overflow --- configure.ac | 2 +- src/app-layer-dnp3.c | 4 ++-- src/detect-bytetest.c | 2 +- src/source-af-packet.c | 2 +- src/suricata-common.h | 6 ++++++ src/util-mem.h | 15 +++++++++++++++ src/util-streaming-buffer.c | 13 +++++++------ 7 files changed, 33 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index b51ab4d47d..ac6fe3d02e 100644 --- a/configure.ac +++ b/configure.ac @@ -207,7 +207,7 @@ AC_FUNC_MMAP AC_FUNC_STRTOD - AC_CHECK_FUNCS([memmem memset memchr memmove]) + AC_CHECK_FUNCS([memmem memset memchr memmove memmove_s]) AC_CHECK_FUNCS([strcasecmp strchr strrchr strdup strndup strncasecmp strtol strstr strpbrk strtoull strtoumax]) AC_CHECK_FUNCS([strerror]) AC_CHECK_FUNCS([gethostname inet_ntoa uname]) diff --git a/src/app-layer-dnp3.c b/src/app-layer-dnp3.c index 41d6b7f011..b3a96127d5 100644 --- a/src/app-layer-dnp3.c +++ b/src/app-layer-dnp3.c @@ -684,8 +684,8 @@ static void DNP3BufferTrim(DNP3Buffer *buffer) DNP3BufferReset(buffer); } else if (buffer->offset > 0) { - memmove(buffer->buffer, buffer->buffer + buffer->offset, - buffer->len - buffer->offset); + SCMemmove(buffer->buffer, buffer->size, buffer->buffer + buffer->offset, + buffer->len - buffer->offset); buffer->len = buffer->len - buffer->offset; buffer->offset = 0; } diff --git a/src/detect-bytetest.c b/src/detect-bytetest.c index 74539c4254..a7ae4c9c20 100644 --- a/src/detect-bytetest.c +++ b/src/detect-bytetest.c @@ -466,7 +466,7 @@ static DetectBytetestData *DetectBytetestParse( end_ptr = (char *)str_ptr; while (!(isspace((unsigned char)*end_ptr) || (*end_ptr == ',')) && (*end_ptr != '\0')) end_ptr++; - memmove(data_offset, str_ptr, end_ptr - str_ptr); + SCMemmove(data_offset, strlen(data_offset) + 1, str_ptr, end_ptr - str_ptr); data_offset[end_ptr-str_ptr] = '\0'; if (data_offset[0] != '-' && isalpha((unsigned char)data_offset[0])) { *offset = SCStrdup(data_offset); diff --git a/src/source-af-packet.c b/src/source-af-packet.c index 1ab466d104..88a672baef 100644 --- a/src/source-af-packet.c +++ b/src/source-af-packet.c @@ -2726,7 +2726,7 @@ static void UpdateRawDataForVLANHdr(Packet *p) uint8_t *pstart = GET_PKT_DATA(p) - VLAN_HEADER_LEN; uint32_t plen = GET_PKT_LEN(p) + VLAN_HEADER_LEN; /* move ethernet addresses */ - memmove(pstart, GET_PKT_DATA(p), 2 * ETH_ALEN); + SCMemmove(pstart, plen, GET_PKT_DATA(p), 2 * ETH_ALEN); /* write vlan info */ *(uint16_t *)(pstart + 2 * ETH_ALEN) = htons(0x8100); *(uint16_t *)(pstart + 2 * ETH_ALEN + 2) = htons(p->afp_v.vlan_tci); diff --git a/src/suricata-common.h b/src/suricata-common.h index 39becde648..d3080f0061 100644 --- a/src/suricata-common.h +++ b/src/suricata-common.h @@ -125,6 +125,12 @@ extern "C" #include #endif +#if HAVE_MEMMOVE_S +#ifndef __STDC_WANT_LIB_EXT1__ +#define __STDC_WANT_LIB_EXT1__ 1 +#endif +#endif + #if HAVE_STRING_H #include #endif diff --git a/src/util-mem.h b/src/util-mem.h index 8ddd2725fb..88268fc354 100644 --- a/src/util-mem.h +++ b/src/util-mem.h @@ -78,4 +78,19 @@ void SCFreeAlignedFunc(void *ptr); #endif /* CPPCHECK */ +#include "util-validate.h" + +static inline void SCMemmove(void *dest, size_t dest_size, const void *src, size_t len) +{ +#if HAVE_MEMMOVE_S + memmove_s(dest, dest_size, src, len); +#else + if (len > dest_size) { + DEBUG_VALIDATE_BUG_ON(1); + len = dest_size; + } + memmove(dest, src, len); +#endif +} + #endif /* SURICATA_UTIL_MEM_H */ diff --git a/src/util-streaming-buffer.c b/src/util-streaming-buffer.c index 02cccc005d..8c96c67818 100644 --- a/src/util-streaming-buffer.c +++ b/src/util-streaming-buffer.c @@ -926,7 +926,8 @@ static inline void StreamingBufferSlideToOffsetWithRegions( // pre-grow: [nextnextnext] // post-grow [nextnextnextXXX] // post-move [XXXnextnextnext] - memmove(next->buf + next_data_offset, next->buf, prev_buf_size); + SCMemmove(next->buf + next_data_offset, next->buf_size - next_data_offset, + next->buf, prev_buf_size); // move portion of "start" into "next" // @@ -966,7 +967,7 @@ static inline void StreamingBufferSlideToOffsetWithRegions( // pre: [xxxxxxxAAA] // post: [AAAxxxxxxx] SCLogDebug("s %u new_data_size %u", s, new_data_size); - memmove(start->buf, start->buf + s, new_data_size); + SCMemmove(start->buf, start->buf_size, start->buf + s, new_data_size); // copy in "next" // pre: [AAAxxxxxxx] @@ -990,7 +991,7 @@ static inline void StreamingBufferSlideToOffsetWithRegions( just_main: SCLogDebug("s %u new_data_size %u", s, new_data_size); - memmove(to_shift->buf, to_shift->buf + s, new_data_size); + SCMemmove(to_shift->buf, to_shift->buf_size, to_shift->buf + s, new_data_size); /* shrink memory region. If this fails we keep the old */ void *ptr = REALLOC(cfg, to_shift->buf, to_shift->buf_size, new_mem_size); if (ptr != NULL) { @@ -1051,7 +1052,7 @@ void StreamingBufferSlideToOffset( const uint32_t size = sb->region.buf_size - slide; SCLogDebug("sliding %u forward, size of original buffer left after slide %u", slide, size); - memmove(sb->region.buf, sb->region.buf + slide, size); + SCMemmove(sb->region.buf, sb->region.buf_size, sb->region.buf + slide, size); if (sb->region.buf_offset > slide) { sb->region.buf_offset -= slide; } else { @@ -1067,7 +1068,7 @@ void StreamingBufferSlideToOffset( const uint32_t size = sb->region.buf_offset - slide; SCLogDebug("sliding %u forward, size of original buffer left after slide %u", slide, size); - memmove(sb->region.buf, sb->region.buf + slide, size); + SCMemmove(sb->region.buf, sb->region.buf_size, sb->region.buf + slide, size); sb->region.stream_offset = offset; sb->region.buf_offset = size; } else { @@ -1299,7 +1300,7 @@ static StreamingBufferRegion *BufferInsertAtRegionConsolidate(StreamingBuffer *s /* validate that the size is exactly what we asked for */ DEBUG_VALIDATE_BUG_ON(dst_size != dst->buf_size); if (dst_copy_offset != 0) - memmove(dst->buf + dst_copy_offset, dst->buf, old_size); + SCMemmove(dst->buf + dst_copy_offset, dst->buf_size - dst_copy_offset, dst->buf, old_size); if (dst_offset != dst->stream_offset) { dst->stream_offset = dst_offset; // buf_offset no longer valid, reset.