Skip to content

Conversation

@MathiasVP
Copy link
Contributor

Just a couple more models that I encountered a need for while doing some Microsoft work

@MathiasVP MathiasVP requested a review from a team as a code owner November 6, 2025 17:04
Copilot AI review requested due to automatic review settings November 6, 2025 17:04
@github-actions github-actions bot added the C++ label Nov 6, 2025
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Nov 6, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds taint flow models for character conversion functions (toupper, tolower) and the iconv function, along with support for additional GCC builtin variants of string manipulation functions. The changes improve taint tracking capabilities for these commonly used functions.

  • Adds YAML models for toupper, tolower, and iconv functions to enable taint tracking
  • Includes test cases demonstrating taint flow through these functions
  • Extends existing string function models to include GCC builtin __builtin___*_chk variants

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
cpp/ql/lib/ext/cctype.model.yml Defines taint flow models for toupper/tolower functions
cpp/ql/lib/ext/iconv.model.yml Defines value flow model for iconv function
cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp Adds test cases for toupper, tolower, and iconv taint tracking
cpp/ql/test/library-tests/dataflow/taint-tests/test_mad-signatures.expected Auto-generated expected test output
cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected Auto-generated expected test output
cpp/ql/lib/semmle/code/cpp/models/implementations/Strcpy.qll Adds support for GCC builtin strcpy/stpcpy/strncpy/stpncpy variants
cpp/ql/lib/semmle/code/cpp/models/implementations/Strcat.qll Adds support for GCC builtin strcat/strncat variants
cpp/ql/lib/semmle/code/cpp/models/implementations/Memset.qll Adds support for GCC builtin memset variant
cpp/ql/lib/semmle/code/cpp/models/implementations/Memcpy.qll Adds support for GCC builtin memmove variant and updates documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"strlcpy", // strlcpy(dst, src, dst_size)
"__builtin___strcpy_chk", // __builtin___strcpy_chk (dest, src, magic);
"__builtin___stpcpy_chk", // __builtin___stpcpy_chk (dest, src, magic);
"__builtin___stpncpy_chk", // __builtin___stpncpy_chk(dest, src, max_amount, magic)
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The comment for __builtin___stpncpy_chk is missing a semicolon at the end, unlike the other builtin function comments on lines 40, 41, and 43. Add a semicolon for consistency.

Suggested change
"__builtin___stpncpy_chk", // __builtin___stpncpy_chk(dest, src, max_amount, magic)
"__builtin___stpncpy_chk", // __builtin___stpncpy_chk(dest, src, max_amount, magic);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we just want to get rid of the semi-colons, similarly for strcat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants