From a5effad9362cc1dfa9b33fea867ac586ca14ca20 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 10:13:39 +0200 Subject: [PATCH] frames: avoid possible undefined behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code analyzer flagged FrameCopy as a possible source of UB due to both pointers passed to memcpy being the same. app-layer-frames.c: In function ‘FrameCopy’: app-layer-frames.c:236:5: warning: overlapping buffers passed as arguments to ‘memcpy’ [-Wanalyzer-overlapping-buffers] 236 | memcpy(dst, src, sizeof(*dst)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ‘FramePrune’: events 1-8 │ │ 750 | static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) │ | ^~~~~~~~~~ │ | | │ | (1) entry to ‘FramePrune’ │...... │ 766 | for (uint16_t i = 0; i < frames->cnt; i++) { │ | ~~~~~~~~~~~~~~~ │ | | │ | (2) following ‘true’ branch... ─>─┐ │ | │ │ | │ │ |┌─────────────────────────────────────────────────────────────┘ │ 767 |│ if (i < FRAMES_STATIC_CNT) { │ |│ ~ │ |│ | │ |└──────────>(3) ...to here │ | (4) following ‘true’ branch (when ‘i <= 2’)... ─>─┐ │ | │ │ | │ │ |┌─────────────────────────────────────────────────────────────┘ │ 768 |│ Frame *frame = &frames->sframes[i]; │ |│ ~~~~~~~~~~~~~~~~~~ │ |│ | │ |└──────────────────────────────────────────>(5) ...to here │ 769 | FrameDebug("prune(s)", frames, frame); │ 770 | if (eof || FrameIsDone(frame, acked)) { │ | ~ │ | | │ | (6) following ‘false’ branch... ─>─┐ │ | │ │...... │ | │ │ |┌──────────────────────────────────────────────────┘ │ 779 |│ const uint64_t fle = FrameLeftEdge(stream, frame); │ |│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ |│ | │ |└────────────────────────────────────>(7) ...to here │ | (8) calling ‘FrameLeftEdge’ from ‘FramePrune’ │ └──> ‘FrameLeftEdge’: event 9 │ │ 257 | static inline uint64_t FrameLeftEdge(const TcpStream *stream, const Frame *frame) │ | ^~~~~~~~~~~~~ │ | | │ | (9) entry to ‘FrameLeftEdge’ │ ‘FrameLeftEdge’: event 10 │ │suricata-common.h:323:27: │ 323 | #define BUG_ON(x) assert(!(x)) │ | ^~~~~~ │ | | │ | (10) following ‘false’ branch (when ‘frame_offset <= app_progress’)... ─>─┐ │ | │ util-validate.h:95:36: note: in expansion of macro ‘BUG_ON’ │ 95 | #define DEBUG_VALIDATE_BUG_ON(exp) BUG_ON((exp)) │ | ^~~~~~ app-layer-frames.c:266:5: note: in expansion of macro ‘DEBUG_VALIDATE_BUG_ON’ │ 266 | DEBUG_VALIDATE_BUG_ON(frame_offset > app_progress); │ | ^~~~~~~~~~~~~~~~~~~~~ │ ‘FrameLeftEdge’: event 11 │ │ | │ │ |┌────────────────────────────────────────────────────────────────────────────────────────────────────┘ │ 269 |│ if (frame->len < 0) { │ |│ ~~~~~^~~~~ │ |│ | │ |└────────────>(11) ...to here │ <──────┘ │ ‘FramePrune’: events 12-13 │ │ 779 | const uint64_t fle = FrameLeftEdge(stream, frame); │ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (12) returning to ‘FramePrune’ from ‘FrameLeftEdge’ │...... │ 783 | FrameCopy(nframe, frame); │ | ~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (13) calling ‘FrameCopy’ from ‘FramePrune’ │ └──> ‘FrameCopy’: events 14-15 │ │ 234 | static void FrameCopy(Frame *dst, Frame *src) │ | ^~~~~~~~~ │ | | │ | (14) entry to ‘FrameCopy’ │ 235 | { │ 236 | memcpy(dst, src, sizeof(*dst)); │ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (15) ⚠️ overlapping buffers passed as arguments to ‘memcpy’ │ In file included from suricata-common.h:129, from app-layer-frames.c:25: /usr/include/string.h:47:14: note: the behavior of ‘memcpy’ is undefined for overlapping buffers 47 | extern void *memcpy (void *__restrict __dest, const void *__restrict __src, | ^~~~~~ --- src/app-layer-frames.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app-layer-frames.c b/src/app-layer-frames.c index 2d79fb463a..3962110414 100644 --- a/src/app-layer-frames.c +++ b/src/app-layer-frames.c @@ -233,6 +233,7 @@ static void FrameClean(Frame *frame) static void FrameCopy(Frame *dst, Frame *src) { + DEBUG_VALIDATE_BUG_ON(dst == src); memcpy(dst, src, sizeof(*dst)); } @@ -353,8 +354,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c #endif } else { Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -378,8 +379,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -780,8 +781,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) le = MIN(le, fle); SCLogDebug("le %" PRIu64 ", frame fle %" PRIu64, le, fle); Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++; @@ -808,8 +809,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++;