Add support for Oracle Call Interface (OCI) to C/C++ coverage

[ebickle]

Description of the issue

The Oracle Call Interface (OCI) is the main low-level C API for Oracle databases. CodeQL lacks coverage for it, particularly for SQL injection sinks.

While I haven't done a robust analysis of the API surface area, the official documentation shows the following functions as accepting SQL input:

• OCIStmtPrepare:
  sword OCIStmtPrepare(OCIStmt* stmtp, OCIError* errhp, const OraText* stmt, ub4 stmt_len, ub4 language, ub4 mode);
• OCIStmtPrepare2:
  sword OCIStmtPrepare2(OCISvcCtx* svchp, OCIStmt** stmthp, OCIError* errhp, const OraText* stmttext, ub4 stmt_len, const OraText* key, ub4 keylen, ub4 language, ub4 mode);

oratypes.h of the OCI (Simple Client) SDK has the following typedefs:

• typedef unsigned char oratext;
• typedef unsigned int ub4;

It looks relatively simple to treat the stmt parameter of OCIStmtPrepare and the stmttext of OCIStmtPrepare2 as SQL sinks within CodeQL. The function names, starting with OCI, are unlikely to collide with other libraries.

I looked at adding this as a PR to expand model coverage, but it looks like the sql-injection sink type isn't supported for C++ CodeQL extension models yet.

[mbg]

Hi @ebickle 👋🏻

Thanks for this feature request and the detailed summary of what you think would be needed. I'll pass this on to the relevant team, but can't make any promises for whether we will be able to officially support this anytime soon.

I looked at adding this as a PR to expand model coverage, but it looks like the sql-injection sink type isn't supported for C++ CodeQL extension models yet.

That does seem to be the case, although we have summary models for SQLite. Combined with the minimal QL code in https://github.com/github/codeql/blob/d83cbde1cb1263fb476a55ea5fd7972307138905/cpp/ql/lib/semmle/code/cpp/models/implementations/SqLite3.qll that's enough to support SQLite. If you are feeling adventurous, you could add a similar implementation for OCI.

[geoffw0]

Hi @ebickle , the QL approach described above (as in SqLite3.qll) should work. Alternatively I believe you could add support for the sql-injection models-as-data sink type in C++ with a very small change to isSink in SqlTainted.ql:

    or
    // sink defined using models-as-data
    sinkNode(node, "sql-injection")

Ideally you would also add at least one model row in a .yml that uses this and one test case alongside to prove everything is working - see the test for this query. Happy to help with any problems that come up if you decide to do this.

[ebickle]

Appreciate all the advice!

I'll look into adding support for sql-injection models-as-data since it would be the most flexible long term and allow for extensibility.

May need some help figuring out how to write appropriate tests for it. Should I add some tests for the new data-as-code support directly into SqlTainted.c, or is there a different preferred approach for testing models?

[geoffw0]

The file you linked is the example (used in the query help), you want to edit the test - either test.c or test.cpp in https://github.com/github/codeql/tree/main/cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted . Then add the .yml model and the edit to SqlTainted.ql as suggested above. Then run the test and accept the results, which will hopefully include your new OCI test case as a result now.

If you get stuck, share your work as far as you got and I'm sure we can unstick you.

[ebickle]

Thanks @geoffw0! I managed to put something together a couple days ago - PR #19832

Surprisingly easy - the team must have done a lot of work for the C/C++ test suites to be that frictionless.

[geoffw0]

Yep, our query tests are pretty mature and usually nice to work with.

There is an issue with your PR - more precisely, with my suggested implementation for adding models-as-data support. I've created a fix for that here: ebickle#1