C++: Support SQL Injection sinks for Oracle Call Interface (OCI) #19832
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for SQL injection sinks for Oracle Call Interface (OCI) in C/C++ by adding models for the OCIStmtPrepare and OCIStmtPrepare2 functions. Key changes include:
- Adding OCI function prototypes and test cases in test.c to trigger both vulnerable (BAD) and safe (GOOD) patterns.
- Updating expected taint flow edges in the SqlTainted.expected file.
- Extending the CodeQL model through modifications in SqlTainted.ql, Oracle.oci.model.yml, and change notes documentation.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/test.c | Added OCI function prototypes and test cases for OCIStmtPrepare and OCIStmtPrepare2 including taint tracking for SQL injection. |
| cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected | Updated expected taint flow edge definitions to include new OCI models. |
| cpp/ql/src/change-notes/2025-06-20-sql-injection-models.md | Documented extension of sql-injection sink support through Models as Data. |
| cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql | Enhanced sink configuration to include the new "sql-injection" model. |
| cpp/ql/lib/ext/Oracle.oci.model.yml | Added sink models for OCIStmtPrepare and OCIStmtPrepare2 functions with sql-injection kind. |
| cpp/ql/lib/change-notes/2025-06-20-oracle-oci-models.md | Documented the addition of OCI sql-injection sink models. |
| } | ||
|
|
||
| // Oracle Call Interface (OCI) Routines | ||
| int OCIStmtPrepare( |
There was a problem hiding this comment.
[nitpick] Consider moving the Oracle OCI function prototypes to a dedicated header file to improve maintainability and reduce duplication in test code.
Copilot uses AI. Check for mistakes.
C++: Fix for the SQL query.
|
Changes LGTM, as does the DCA run. Looks like a valuable extension. Not sure why three CI checks are failing, it doesn't look like it has anything to do with the changes here. I'll try rerunning them. @jketema would you might checking this and approving, since I wrote some of the changes so it would be good to have a third opinion here. |
Looks like this has to do with some internal changes we made. I've merged in the latest main, which hopefully resolves this. |
|
One remaining test failure. This is a genuine one for |
|
Oh that's annoying, it's a test that outputs the MaD row IDs (which are not stable when models are added). @ebickle do you know how to accept the changes to @jketema can we remove the MaD IDs from this test, or pretty print them like we do in query tests so that we get more stable output? |
Do you have an example of the pretty printing? |
Rust I'm not sure if this is available in C++, nor if it would really do the right thing in this case. |
I should be able to accept the changes ( With a synced fork and the latest CodeQL binaries I keep getting this on random unit tests, including the flow test but not CWE-089:
This happened on a machine with a fresh install of CodeQL so the "database is not compatible" message likely isn't something on my end. I've tried updating the CodeQL binaries again and nuking my profile's I've also tossed an invite to my fork over to the two of you in case you need it. |
|
I was able to run the test in VS Code - the update for the |
I think that means it's picking up an old version of the CodeQL C++ library, which doesn't have the necessary downgrade scripts. VSCode apparently picks up the right version automatically. |
Thanks. That looks doable. I'll have a look once this PR is merged. |
|
Merged. @ebickle thanks for your contribution! |
Fixes #19764
sql-injectionModels as Data (MaD) sink kind for C/C++.sql-injectionsink models for the Oracle Call Interface (OCI) database library functionsOCIStmtPrepareandOCIStmtPrepare2.