object_detect_udp_stage: fix name buffer overread and GCC 12 false positive#918
Open
Kletternaut wants to merge 2 commits into
raspberrypi:mainraspberrypi/rpicam-apps:mainfrom
Kletternaut:fix/object-detect-udp-name-bufferKletternaut/rpicam-apps:fix/object-detect-udp-name-bufferCopy head branch name to clipboard
Open
object_detect_udp_stage: fix name buffer overread and GCC 12 false positive#918Kletternaut wants to merge 2 commits intoraspberrypi:mainraspberrypi/rpicam-apps:mainfrom Kletternaut:fix/object-detect-udp-name-bufferKletternaut/rpicam-apps:fix/object-detect-udp-name-bufferCopy head branch name to clipboard
Kletternaut wants to merge 2 commits into
raspberrypi:mainraspberrypi/rpicam-apps:mainfrom
Kletternaut:fix/object-detect-udp-name-bufferKletternaut/rpicam-apps:fix/object-detect-udp-name-bufferCopy head branch name to clipboard
Conversation
…sitive The name field was unconditionally copied using a fixed length of 253 bytes regardless of the actual string length. If detection.name is shorter than 253 characters the memcpy reads past the end of the string data, causing undefined behaviour. Fix by zero-initialising the name array and limiting the copy to min(name.size(), name_length - 1) bytes. The trailing NUL terminator is already provided by the zero-initialisation. GCC 12 additionally emits a spurious -Wstringop-overflow / -Warray-bounds diagnostic originating from vector::resize() inlined inside the append() helper when T = std::array<uint8_t,255>. The accesses are entirely within bounds; suppress the false positive with a narrow diagnostic pragma around the template definition. Signed-off-by: Kletternaut <tomge68@gmail.com>
Clang does not recognise -Wstringop-overflow and rejects the pragma with -Werror,-Wunknown-warning-option, breaking the clang CI build. Wrap the diagnostic push/pop pair in a #if defined(__GNUC__) && !defined(__clang__) guard so the pragmas are only seen by GCC. Signed-off-by: Kletternaut <tomge68@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
ObjectDetectUDPStage::Process()the name field of each detection was serialised with a hard-codedmemcpyof 253 bytes:Three bugs:
detection.nameis typically a short label (e.g."person", 6 bytes).memcpyalways reads 253 bytes, reading well past the end of the string — undefined behaviour.name.size()and byte 253 contain stack garbage sent over the UDP socket.append<std::array<uint8_t,255>>triggers a spurious-Wstringop-overflow/-Warray-boundsdiagnostic fromvector::resize()inlined deep instl_algobase.h. The access is entirely within bounds; GCC's alias analysis loses track of the allocation origin.Fix
name{}).min(detection.name.size(), name_length - 1)bytes.name[254] = '\0'needed.#pragma GCC diagnosticpair around theappendtemplate definition, with a comment explaining the root cause.The on-wire format is unchanged — the receiver still reads exactly
name_length(255) bytes for the name field.Testing
Compiled with GCC 12.2.0, -O3 -Wall -Wextra -Warray-bounds=2 (OpenCV enabled) — no warnings.