Skip to content

C++: Add summary models for openssl and sqlite #19492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented May 14, 2025

Now that #19443 is merged the model generator gives the models I was looking for in the case of OpenSSL 🎉 So I think this is a good initial set of models.

Commit-by-commit review recommended since I did a small change in fd768ae to filter out which functions we want to generate models for based on the results form the first two commits, and then I redid the models in the next commit.

Note that there are lots of spurious results for the dubious signature consistency check. This check is actually not really representative of the signatures we use in models for C++. In particular, the dubious signature is designed to match signatures like (a,b,c) (with no spaces), but in C++ we can have spaces because of derived types such as (a *,b &,c **), etc. We could probably design a better check for dubious signatures, but I don't really want to get into that right now.

@github-actions github-actions bot added the C++ label May 14, 2025
@MathiasVP MathiasVP marked this pull request as ready for review May 14, 2025 12:46
@Copilot Copilot AI review requested due to automatic review settings May 14, 2025 12:46
@MathiasVP MathiasVP requested a review from a team as a code owner May 14, 2025 12:46
Copy link
Contributor

@Copilot 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

Adds initial flow summary models for the OpenSSL and SQLite libraries, filters out functions that already have hand-written models or aren’t part of project sources, and updates test expectations accordingly.

  • Updated expected test outputs to use new provenance IDs
  • Extended CaptureModels.qll to skip existing taint/dataflow models, main, and system headers
  • Added a change-note entry announcing the new library models

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

File Description
cpp/ql/test/library-tests/dataflow/external-models/flow.expected Updated provenance IDs in expected test output
cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll Added imports and filtering predicates to skip pre-modeled/system functions
cpp/ql/src/change-notes/2025-05-14-openssl-sqlite-models.md Logged the addition of OpenSSL and SQLite flow models
Comments suppressed due to low confidence (1)

cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll:17

  • [nitpick] The alias DataFlow duplicates the module and type name, which may cause confusion when referencing DataFlowFunction. Consider renaming the alias (e.g., DataFlowModels) for clarity.
private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow

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

Successfully merging this pull request may close these issues.

1 participant