Raising the bar with custom CodeQL
Code scanning is a straightforward addition to any GitHub repository: a prebuilt Action workflow runs CodeQL's default set of security queries against your code and files alerts as pull request checks. Exiv2 enabled it early, but found value in moving beyond the defaults. The curated suite is deliberately conservative to keep alert noise low, and the open-source CodeQL repository holds many more queries that aren't enabled out of the box. Projects that want a deeper pass can point their config at extra query packs.
Exiv2 did exactly that. Its workflow YAML contains a line that pulls in an additional configuration file:
config-file: .github/codeql/codeql-config.yml
That file, codeql-config.yml, lists the extra checks the project runs on every push and pull request.
Writing checks for your own blind spots
The real payoff comes from writing queries that encode lessons from your own vulnerability history. CodeQL's language lets you treat code as data, so you can express a bug pattern in a few lines and then sweep the entire codebase for every variation of it. The Exiv2 team has written a handful of such queries, including a deliberately simple one that flags signed shifts. The most elaborate one, unsafe_vector_access.ql, is more instructive because it exercises advanced CodeQL features and also illustrates a key design tension: a general-purpose query has to be cautious about false positives, but a query scoped to a single codebase can be much more aggressive—and still be a net win for that project.
The query looks for uses of std::vector::operator[] that could read or write past the end of the vector. That API doesn't do bounds checking; it's fast by design, and mistakes surface only later as corruption or crashes. Historically, that class of bug has caused real security issues in Exiv2, which is exactly why the team wanted an automated guard.
Refining the unsafe-index query
The initial version of the query — a bare search for every std::vector::operator[] call — returned 402 results on Exiv2 0.27.4, almost all of them false positives. Flagging every call and asking the team to switch to at() would create unwanted churn, since most of those indices are provably safe in context. The final query therefore adds several filtering clauses. For clarity, the version discussed here omits a few minor details from the production query, but the important logic is unchanged.
/**
* @name Unsafe vector access
* @description std::vector::operator[] does not do any runtime
* bounds-checking, so it is safer to use std::vector::at()
* @kind problem
* @problem.severity warning
* @id cpp/unsafe-vector-access
* @tags security
* external/cwe/cwe-125
*/
import cpp
// A call to `operator[]`.
class ArrayIndexCall extends FunctionCall {
ClassTemplateInstantiation ti;
TemplateClass tc;
ArrayIndexCall() {
this.getTarget().getName() = "operator[]" and
ti = this.getQualifier().getType().getUnspecifiedType() and
tc = ti.getTemplate() and
tc.getSimpleName() != "map"
}
ClassTemplateInstantiation getClassTemplateInstantiation() { result = ti }
}
from ArrayIndexCall call
select call, "Unsafe use of operator[]. Use the at() method instead."
The ArrayIndexCall class matches overloaded operator[] calls as FunctionCalls, which covers types like std::vector and std::string but excludes built-in array indexing. std::map is explicitly filtered out, since a missing key fails gracefully rather than crashing. The added filter clauses are:
from ArrayIndexCall call
where
// Ignore results in the xmpsdk directory.
not call.getLocation().getFile().getRelativePath().matches("xmpsdk/%") and
// Ignore accesses like this: `vsnprintf(&buffer[0], buffer.size(), format, args)`
// That's pointer arithmetic, not a deref, so it's usually a false positive.
not exists(AddressOfExpr addrExpr | addrExpr.getOperand() = call) and
not indexK_with_check(_, call) and
not indexI_with_check(_, call) and
not index_last_with_check(_, call)
select call, "Unsafe use of operator[]. Use the at() method instead."
Crude exclusions
The first filter is the simplest: results inside the xmpsdk subdirectory are dropped entirely.
// Ignore results in the xmpsdk directory.
not call.getLocation().getFile().getRelativePath().matches("xmpsdk/%")
That directory contains a vendored copy of the Adobe XMP SDK, added years ago when the SDK was distributed only as a tarball. Although Adobe now publishes the SDK on GitHub, the migration to a submodule hasn’t happened, so Exiv2 tries to keep changes to that third-party code minimal. Flagging issues there would only invite unnecessary edits to code the team prefers not to touch.
The second heuristic removes cases where the result of operator[] is immediately passed to AddressOfExpr:
// Ignore accesses like this: `vsnprintf(&buffer[0], buffer.size(), format, args)`
// That's pointer arithmetic, not a deref, so it's usually a false positive.
not exists(AddressOfExpr addrExpr | addrExpr.getOperand() = call) and
When an address is taken, the element isn’t dereferenced at that point. An out-of-bounds index could still cause problems, but determining that would require deeper data-flow analysis than this query attempts, so those results are simply skipped.
Constant indices with guards
A more principled filter handles constants that are provably in range. An access like numbers[9] may look suspicious, but it is safe when guarded by a prior bounds check on the array’s size:
if(numbers.size()>=10)
The query eliminates those cases with indexK_with_check, which confirms that a lower-bound condition on the vector’s size controls the block containing the indexing expression:
guard.controls(block, branch) and
That condition comes from minimum_size_cond, a predicate that recognizes idioms like x.size() > 2 or !x.empty(). The Guards library establishes control flow, while GlobalValueNumbering ensures the vector named in the condition is the same one being indexed. That last check prevents false negatives like this:
if (x.size() > 2) { ... y[2] ... }
Symbolic indices with bounds checks
The most common safe pattern in Exiv2 indexes with a variable after checking it against size(), as in this snippet from tags_int.cpp:
for (size_t i = 0; i < stringValue.length(); ++i) {
if (stringValue[i] == 'T') stringValue[i] = ' ';
if (stringValue[i] == '-') stringValue[i] = ':';
}
The corresponding predicate, indexInBounds_check, works like the constant-index version, but uses GlobalValueNumbering a second time to verify the symbolic index expression in the guard matches the one used in the indexing operation:
// Array accesses like this are safe:
// `if (i < x.size()) { ... x[i] ... }`
predicate indexI_with_check(GuardCondition guard, ArrayIndexCall call) {
exists(Expr idx, SizeCall sizeCall, BasicBlock block, boolean branch |
relOpWithSwapAndNegate(guard, idx, sizeCall, Lesser(), Strict(), branch) and
globalValueNumber(sizeCall.getQualifier()) = globalValueNumber(call.getQualifier()) and
globalValueNumber(idx) = globalValueNumber(call.getArgument(0)) and
guard.controls(block, branch) and
block.contains(call)
)
}
Indexing the last element
Another frequent idiom indexes x[x.size() - 1]. That is safe whenever the vector is non-empty. The query has a dedicated clause for this pattern:
not index_last_with_check(_, call)
That clause relies on indexIsLast_with_check, which mirrors the other predicates but needs extra logic to match the x.size()-1 expression. It also introduces DataFlow::localExprFlow to handle a variant where the size is first copied into a local variable:
// pop trailing ':' on a namespace
if ( bNS && !out.empty() ) {
std::size_t length = out.length();
if ( out[length-1] == ':' ) out = out.substr(0,length-1);
}
Without that data-flow step, the query would miss cases where the bounds check and the index expression reference different variables holding the same value.
Why custom queries pay off
The resulting query is deliberately narrow. It recognizes the idioms that actually appear in Exiv2 — guarded constant indices, guarded symbolic indices, and last-element access — and ignores patterns it cannot analyze, like indexing with an unconstrained variable. A general-purpose rule would need to be far more conservative or else would drown the team in false positives. Since it targets this codebase, it can be strict enough to be useful.
Not every custom query needs this level of complexity. A separate Exiv2 query that flags signed shifts, another source of undefined behavior, is far simpler. Both were motivated by real vulnerabilities: issue #1706 traced back to an out-of-bounds operator[] call, and the signed-shift query came from a similar past bug. With code scanning enabled on pull requests, those variants are caught before they reach the default branch.
Building a test database
Query development is easiest with the CodeQL extension for VS Code plus a database for the target codebase. A prebuilt Exiv2 0.27.4 database is available, or you can build one with the CodeQL CLI:
# Get the CodeQL CLI
export CODEQL_VERSION=v2.6.1
export EXIV2_VERSION=v0.27.4
mkdir codeql-home
cd codeql-home
curl -L -O https://github.com/github/codeql-cli-binaries/releases/download/$CODEQL_VERSION/codeql-linux64.zip
unzip codeql-linux64.zip
# Get the CodeQL queries
git clone https://github.com/github/codeql.git codeql-queries
cd ..
# Build a database for Exiv2
git clone https://github.com/Exiv2/exiv2.git
cd exiv2/
git checkout $EXIV2_VERSION
../codeql-home/codeql/codeql database create exiv2_$EXIV2_VERSION --language=cpp
zip -r exiv2_$EXIV2_VERSION.zip exiv2_$EXIV2_VERSION/
Those commands are for Linux but adapt easily to macOS or Windows. Use the latest CLI release, but keep Exiv2 at version 0.27.4: the query results are cleaner there, since later releases have already fixed most of the flagged code.
