Skip to content

C++: Fix CWE-119 memcpy tests - #20876

Merged
geoffw0 merged 3 commits into
github:mainfrom
szsam:fix-CWE-119-tests
Nov 27, 2025
Merged

C++: Fix CWE-119 memcpy tests#20876
geoffw0 merged 3 commits into
github:mainfrom
szsam:fix-CWE-119-tests

Conversation

@szsam

@szsam szsam commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

sizeof(pointer) only gives the pointer size, not the buffer size, so use explicit 10/20 lengths in tests.cpp and update OverflowBuffer.expected to accept the resulting memcpy diagnostics.

sizeof(pointer) only gives the pointer size, not the buffer
size, so use explicit 10/20 lengths in tests.cpp and update
OverflowBuffer.expected to accept the resulting memcpy diagnostics.

Signed-off-by: Mingjie Shen <shen497@purdue.edu>
@szsam
szsam requested a review from a team as a code owner November 19, 2025 22:10
@github-actions github-actions Bot added the C++ label Nov 19, 2025

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing this. We would like to be a bit more explicit, see below. Otherwise this looks fine.

Comment on lines +33 to +36
memcpy(bigbuffer, smallbuffer, 10); // GOOD
memcpy(bigbuffer, smallbuffer, 20); // BAD: over-read
memcpy(smallbuffer, bigbuffer, 10); // GOOD
memcpy(smallbuffer, bigbuffer, 20); // BAD: over-write

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would prefer the following here:

Suggested change
memcpy(bigbuffer, smallbuffer, 10); // GOOD
memcpy(bigbuffer, smallbuffer, 20); // BAD: over-read
memcpy(smallbuffer, bigbuffer, 10); // GOOD
memcpy(smallbuffer, bigbuffer, 20); // BAD: over-write
memcpy(bigbuffer, smallbuffer, sizeof(char) * 10); // GOOD
memcpy(bigbuffer, smallbuffer, sizeof(char) * 20); // BAD: over-read
memcpy(smallbuffer, bigbuffer, sizeof(char) * 10); // GOOD
memcpy(smallbuffer, bigbuffer, sizeof(char) * 20); // BAD: over-write

Comment thread cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp Outdated

@geoffw0 geoffw0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

@geoffw0
geoffw0 merged commit 7db06ca into github:main Nov 27, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants