- name: Enable possibly useful queries which are disabled by default
uses: ./.github/codeql-custom.qls
- name: systemd-specific CodeQL queries
- uses: ./.lgtm/cpp-queries/
+ uses: ./.github/codeql-queries/
--- /dev/null
+/**
+ * vi: sw=2 ts=2 et syntax=ql:
+ *
+ * Borrowed from
+ * https://github.com/Semmle/ql/blob/master/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql
+ *
+ * @name Use of potentially dangerous function
+ * @description Certain standard library functions are dangerous to call.
+ * @id cpp/potentially-dangerous-function
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags reliability
+ * security
+ */
+import cpp
+
+predicate potentiallyDangerousFunction(Function f, string message) {
+ (
+ f.getQualifiedName() = "fgets" and
+ message = "Call to fgets() is potentially dangerous. Use read_line() instead."
+ ) or (
+ f.getQualifiedName() = "strtok" and
+ message = "Call to strtok() is potentially dangerous. Use extract_first_word() instead."
+ ) or (
+ f.getQualifiedName() = "strsep" and
+ message = "Call to strsep() is potentially dangerous. Use extract_first_word() instead."
+ ) or (
+ f.getQualifiedName() = "dup" and
+ message = "Call to dup() is potentially dangerous. Use fcntl(fd, FD_DUPFD_CLOEXEC, 3) instead."
+ ) or (
+ f.getQualifiedName() = "htonl" and
+ message = "Call to htonl() is confusing. Use htobe32() instead."
+ ) or (
+ f.getQualifiedName() = "htons" and
+ message = "Call to htons() is confusing. Use htobe16() instead."
+ ) or (
+ f.getQualifiedName() = "ntohl" and
+ message = "Call to ntohl() is confusing. Use be32toh() instead."
+ ) or (
+ f.getQualifiedName() = "ntohs" and
+ message = "Call to ntohs() is confusing. Use be16toh() instead."
+ ) or (
+ f.getQualifiedName() = "strerror" and
+ message = "Call to strerror() is not thread-safe. Use strerror_r() or printf()'s %m format string instead."
+ ) or (
+ f.getQualifiedName() = "accept" and
+ message = "Call to accept() is not O_CLOEXEC-safe. Use accept4() instead."
+ ) or (
+ f.getQualifiedName() = "dirname" and
+ message = "Call dirname() is icky. Use path_extract_directory() instead."
+ )
+}
+
+from FunctionCall call, Function target, string message
+where
+ call.getTarget() = target and
+ potentiallyDangerousFunction(target, message)
+select call, message
--- /dev/null
+/**
+ * vi: sw=2 ts=2 et syntax=ql:
+ *
+ * Based on cpp/uninitialized-local.
+ *
+ * @name Potentially uninitialized local variable using the cleanup attribute
+ * @description Running the cleanup handler on a possibly uninitialized variable
+ * is generally a bad idea.
+ * @id cpp/uninitialized-local-with-cleanup
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import semmle.code.cpp.controlflow.StackVariableReachability
+
+/** Auxiliary predicate: List cleanup functions we want to explicitly ignore
+ * since they don't do anything illegal even when the variable is uninitialized
+ */
+predicate cleanupFunctionDenyList(string fun) {
+ fun = "erase_char"
+}
+
+/**
+ * A declaration of a local variable using __attribute__((__cleanup__(x)))
+ * that leaves the variable uninitialized.
+ */
+DeclStmt declWithNoInit(LocalVariable v) {
+ result.getADeclaration() = v and
+ not v.hasInitializer() and
+ /* The variable has __attribute__((__cleanup__(...))) set */
+ v.getAnAttribute().hasName("cleanup") and
+ /* Check if the cleanup function is not on a deny list */
+ not cleanupFunctionDenyList(v.getAnAttribute().getAnArgument().getValueText())
+}
+
+class UninitialisedLocalReachability extends StackVariableReachability {
+ UninitialisedLocalReachability() { this = "UninitialisedLocal" }
+
+ override predicate isSource(ControlFlowNode node, StackVariable v) { node = declWithNoInit(v) }
+
+ /* Note: _don't_ use the `useOfVarActual()` predicate here (and a couple of lines
+ * below), as it assumes that the callee always modifies the variable if
+ * it's passed to the function.
+ *
+ * i.e.:
+ * _cleanup_free char *x;
+ * fun(&x);
+ * puts(x);
+ *
+ * `useOfVarActual()` won't treat this an an uninitialized read even if the callee
+ * doesn't modify the argument, however, `useOfVar()` will
+ */
+ override predicate isSink(ControlFlowNode node, StackVariable v) { useOfVar(v, node) }
+
+ override predicate isBarrier(ControlFlowNode node, StackVariable v) {
+ // only report the _first_ possibly uninitialized use
+ useOfVar(v, node) or
+ (
+ /* If there's an return statement somewhere between the variable declaration
+ * and a possible definition, don't accept is as a valid initialization.
+ *
+ * E.g.:
+ * _cleanup_free_ char *x;
+ * ...
+ * if (...)
+ * return;
+ * ...
+ * x = malloc(...);
+ *
+ * is not a valid initialization, since we might return from the function
+ * _before_ the actual iniitialization (emphasis on _might_, since we
+ * don't know if the return statement might ever evaluate to true).
+ */
+ definitionBarrier(v, node) and
+ not exists(ReturnStmt rs |
+ /* The attribute check is "just" a complexity optimization */
+ v.getFunction() = rs.getEnclosingFunction() and v.getAnAttribute().hasName("cleanup") |
+ rs.getLocation().isBefore(node.getLocation())
+ )
+ )
+ }
+}
+
+pragma[noinline]
+predicate containsInlineAssembly(Function f) { exists(AsmStmt s | s.getEnclosingFunction() = f) }
+
+/**
+ * Auxiliary predicate: List common exceptions or false positives
+ * for this check to exclude them.
+ */
+VariableAccess commonException() {
+ // If the uninitialized use we've found is in a macro expansion, it's
+ // typically something like va_start(), and we don't want to complain.
+ result.getParent().isInMacroExpansion()
+ or
+ result.getParent() instanceof BuiltInOperation
+ or
+ // Finally, exclude functions that contain assembly blocks. It's
+ // anyone's guess what happens in those.
+ containsInlineAssembly(result.getEnclosingFunction())
+}
+
+from UninitialisedLocalReachability r, LocalVariable v, VariableAccess va
+where
+ r.reaches(_, v, va) and
+ not va = commonException()
+select va, "The variable $@ may not be initialized here, but has a cleanup handler.", v, v.getName()
--- /dev/null
+---
+# vi: ts=2 sw=2 et syntax=yaml:
+# SPDX-License-Identifier: LGPL-2.1-or-later
+
+library: false
+name: systemd/cpp-queries
+version: 0.0.1
+dependencies:
+ codeql/cpp-all: "*"
+ codeql/suite-helpers: "*"
+extractor: cpp
+++ /dev/null
----
-# vi: ts=2 sw=2 et:
-# SPDX-License-Identifier: LGPL-2.1-or-later
-
-# Explicitly enable certain checks which are hidden by default
-queries:
- - include: cpp/bad-strncpy-size
- - include: cpp/declaration-hides-variable
- - include: cpp/inconsistent-null-check
- - include: cpp/mistyped-function-arguments
- - include: cpp/nested-loops-with-same-variable
- - include: cpp/sizeof-side-effect
- - include: cpp/suspicious-pointer-scaling
- - include: cpp/suspicious-pointer-scaling-void
- - include: cpp/suspicious-sizeof
- - include: cpp/unsafe-strcat
- - include: cpp/unsafe-strncat
- - include: cpp/unsigned-difference-expression-compared-zero
- - include: cpp/unused-local-variable
- - include:
- tags:
- - "security"
- - "correctness"
- severity: "error"
-
-extraction:
- cpp:
- prepare:
- packages:
- - libpwquality-dev
- - libfdisk-dev
- - libp11-kit-dev
- - libssl-dev
- - python3-jinja2
- after_prepare:
- - pip3 install -r .github/workflows/requirements.txt --require-hashes
- - export PATH="/opt/work/.local/bin:$PATH"
- python:
- python_setup:
- version: 3
+++ /dev/null
-/**
- * vi: sw=2 ts=2 et syntax=ql:
- *
- * Borrowed from
- * https://github.com/Semmle/ql/blob/master/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql
- *
- * @name Use of potentially dangerous function
- * @description Certain standard library functions are dangerous to call.
- * @id cpp/potentially-dangerous-function
- * @kind problem
- * @problem.severity error
- * @precision high
- * @tags reliability
- * security
- */
-import cpp
-
-predicate potentiallyDangerousFunction(Function f, string message) {
- (
- f.getQualifiedName() = "fgets" and
- message = "Call to fgets() is potentially dangerous. Use read_line() instead."
- ) or (
- f.getQualifiedName() = "strtok" and
- message = "Call to strtok() is potentially dangerous. Use extract_first_word() instead."
- ) or (
- f.getQualifiedName() = "strsep" and
- message = "Call to strsep() is potentially dangerous. Use extract_first_word() instead."
- ) or (
- f.getQualifiedName() = "dup" and
- message = "Call to dup() is potentially dangerous. Use fcntl(fd, FD_DUPFD_CLOEXEC, 3) instead."
- ) or (
- f.getQualifiedName() = "htonl" and
- message = "Call to htonl() is confusing. Use htobe32() instead."
- ) or (
- f.getQualifiedName() = "htons" and
- message = "Call to htons() is confusing. Use htobe16() instead."
- ) or (
- f.getQualifiedName() = "ntohl" and
- message = "Call to ntohl() is confusing. Use be32toh() instead."
- ) or (
- f.getQualifiedName() = "ntohs" and
- message = "Call to ntohs() is confusing. Use be16toh() instead."
- ) or (
- f.getQualifiedName() = "strerror" and
- message = "Call to strerror() is not thread-safe. Use strerror_r() or printf()'s %m format string instead."
- ) or (
- f.getQualifiedName() = "accept" and
- message = "Call to accept() is not O_CLOEXEC-safe. Use accept4() instead."
- ) or (
- f.getQualifiedName() = "dirname" and
- message = "Call dirname() is icky. Use path_extract_directory() instead."
- )
-}
-
-from FunctionCall call, Function target, string message
-where
- call.getTarget() = target and
- potentiallyDangerousFunction(target, message)
-select call, message
+++ /dev/null
-/**
- * vi: sw=2 ts=2 et syntax=ql:
- *
- * Based on cpp/uninitialized-local.
- *
- * @name Potentially uninitialized local variable using the cleanup attribute
- * @description Running the cleanup handler on a possibly uninitialized variable
- * is generally a bad idea.
- * @id cpp/uninitialized-local-with-cleanup
- * @kind problem
- * @problem.severity error
- * @precision high
- * @tags security
- */
-
-import cpp
-import semmle.code.cpp.controlflow.StackVariableReachability
-
-/** Auxiliary predicate: List cleanup functions we want to explicitly ignore
- * since they don't do anything illegal even when the variable is uninitialized
- */
-predicate cleanupFunctionDenyList(string fun) {
- fun = "erase_char"
-}
-
-/**
- * A declaration of a local variable using __attribute__((__cleanup__(x)))
- * that leaves the variable uninitialized.
- */
-DeclStmt declWithNoInit(LocalVariable v) {
- result.getADeclaration() = v and
- not v.hasInitializer() and
- /* The variable has __attribute__((__cleanup__(...))) set */
- v.getAnAttribute().hasName("cleanup") and
- /* Check if the cleanup function is not on a deny list */
- not cleanupFunctionDenyList(v.getAnAttribute().getAnArgument().getValueText())
-}
-
-class UninitialisedLocalReachability extends StackVariableReachability {
- UninitialisedLocalReachability() { this = "UninitialisedLocal" }
-
- override predicate isSource(ControlFlowNode node, StackVariable v) { node = declWithNoInit(v) }
-
- /* Note: _don't_ use the `useOfVarActual()` predicate here (and a couple of lines
- * below), as it assumes that the callee always modifies the variable if
- * it's passed to the function.
- *
- * i.e.:
- * _cleanup_free char *x;
- * fun(&x);
- * puts(x);
- *
- * `useOfVarActual()` won't treat this an an uninitialized read even if the callee
- * doesn't modify the argument, however, `useOfVar()` will
- */
- override predicate isSink(ControlFlowNode node, StackVariable v) { useOfVar(v, node) }
-
- override predicate isBarrier(ControlFlowNode node, StackVariable v) {
- // only report the _first_ possibly uninitialized use
- useOfVar(v, node) or
- (
- /* If there's an return statement somewhere between the variable declaration
- * and a possible definition, don't accept is as a valid initialization.
- *
- * E.g.:
- * _cleanup_free_ char *x;
- * ...
- * if (...)
- * return;
- * ...
- * x = malloc(...);
- *
- * is not a valid initialization, since we might return from the function
- * _before_ the actual iniitialization (emphasis on _might_, since we
- * don't know if the return statement might ever evaluate to true).
- */
- definitionBarrier(v, node) and
- not exists(ReturnStmt rs |
- /* The attribute check is "just" a complexity optimization */
- v.getFunction() = rs.getEnclosingFunction() and v.getAnAttribute().hasName("cleanup") |
- rs.getLocation().isBefore(node.getLocation())
- )
- )
- }
-}
-
-pragma[noinline]
-predicate containsInlineAssembly(Function f) { exists(AsmStmt s | s.getEnclosingFunction() = f) }
-
-/**
- * Auxiliary predicate: List common exceptions or false positives
- * for this check to exclude them.
- */
-VariableAccess commonException() {
- // If the uninitialized use we've found is in a macro expansion, it's
- // typically something like va_start(), and we don't want to complain.
- result.getParent().isInMacroExpansion()
- or
- result.getParent() instanceof BuiltInOperation
- or
- // Finally, exclude functions that contain assembly blocks. It's
- // anyone's guess what happens in those.
- containsInlineAssembly(result.getEnclosingFunction())
-}
-
-from UninitialisedLocalReachability r, LocalVariable v, VariableAccess va
-where
- r.reaches(_, v, va) and
- not va = commonException()
-select va, "The variable $@ may not be initialized here, but has a cleanup handler.", v, v.getName()
+++ /dev/null
----
-# vi: ts=2 sw=2 et syntax=yaml:
-# SPDX-License-Identifier: LGPL-2.1-or-later
-
-library: false
-name: systemd/cpp-queries
-version: 0.0.1
-dependencies:
- codeql/cpp-all: "*"
- codeql/suite-helpers: "*"
-extractor: cpp
projects / systemd / .git / commitdiff