Merge branch 'main' into knewbury01/more-FP-fix

Author: lcartey

.github/workflows/dispatch-matrix-test-on-comment.yml

@@ -35,7 +35,7 @@ jobs:
           token: ${{ secrets.RELEASE_ENGINEERING_TOKEN }}
           repository: github/codeql-coding-standards-release-engineering
           event-type: matrix-test
-          client-payload: '{"pr": "${{ github.event.number  }}"}'
+          client-payload: '{"pr": "${{ github.event.issue.number }}"}'
 
       - uses: actions/github-script@v6
         if: ${{ github.event.issue.pull_request && contains(github.event.comment.body, '/test-matrix') }}

.vscode/tasks.json

@@ -249,6 +249,7 @@
                 "Null",
                 "OperatorInvariants",
                 "Operators",
+                "OutOfBounds",
                 "Pointers",
                 "Pointers1",
                 "Pointers2",

README.md

@@ -50,3 +50,5 @@ All header files in [c/common/test/includes/standard-library](./c/common/test/in
 ---
 
 <sup>1</sup>This repository incorporates portions of the SEI CERT® Coding Standards available at https://wiki.sei.cmu.edu/confluence/display/seccode/SEI+CERT+Coding+Standards; however, such use does not necessarily constitute or imply an endorsement, recommendation, or favoring by Carnegie Mellon University or its Software Engineering Institute.
+
+

c/cert/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cert-c-coding-standards
-version: 2.15.0-dev
+version: 2.18.0-dev
 description: CERT C 2016
 suites: codeql-suites
 license: MIT

c/cert/src/rules/ARR30-C/DoNotFormOutOfBoundsPointersOrArraySubscripts.md

@@ -0,0 +1,52 @@
+/**
+ * @id c/cert/do-not-form-out-of-bounds-pointers-or-array-subscripts
+ * @name ARR30-C: Do not form or use out-of-bounds pointers or array subscripts
+ * @description Forming or using an out-of-bounds pointer is undefined behavior and can result in
+ *              invalid memory accesses.
+ * @kind problem
+ * @precision medium
+ * @problem.severity error
+ * @tags external/cert/id/arr30-c
+ *       correctness
+ *       security
+ *       external/cert/obligation/rule
+ */
+
+ import cpp
+ import codingstandards.c.cert
+ import codingstandards.c.OutOfBounds
+ 
+ from
+   OOB::BufferAccess ba, Expr bufferArg, Expr sizeArg, OOB::PointerToObjectSource bufferSource,
+   string message
+ where
+   not isExcluded(ba, OutOfBoundsPackage::doNotFormOutOfBoundsPointersOrArraySubscriptsQuery()) and
+   // exclude loops
+   not exists(Loop loop | loop.getStmt().getChildStmt*() = ba.getEnclosingStmt()) and
+   // exclude size arguments that are of type ssize_t
+   not sizeArg.getAChild*().(VariableAccess).getTarget().getType() instanceof Ssize_t and
+   // exclude size arguments that are assigned the result of a function call e.g. ftell
+   not sizeArg.getAChild*().(VariableAccess).getTarget().getAnAssignedValue() instanceof FunctionCall and
+   // exclude field or array accesses for the size arguments
+   not sizeArg.getAChild*() instanceof FieldAccess and
+   not sizeArg.getAChild*() instanceof ArrayExpr and
+   (
+     exists(int sizeArgValue, int bufferArgSize |
+      OOB::isSizeArgGreaterThanBufferSize(bufferArg, sizeArg, bufferSource, bufferArgSize, sizeArgValue, ba) and
+       message =
+         "Buffer accesses offset " + sizeArgValue + 
+         " which is greater than the fixed size " + bufferArgSize + " of the $@."
+     )
+     or
+     exists(int sizeArgUpperBound, int sizeMult, int bufferArgSize |
+       OOB::isSizeArgNotCheckedLessThanFixedBufferSize(bufferArg, sizeArg, bufferSource,
+         bufferArgSize, ba, sizeArgUpperBound, sizeMult) and
+       message =
+         "Buffer may access up to offset " + sizeArgUpperBound + "*" + sizeMult +
+           " which is greater than the fixed size " + bufferArgSize + " of the $@."
+     )
+     or
+     OOB::isSizeArgNotCheckedGreaterThanZero(bufferArg, sizeArg, bufferSource, ba) and
+     message = "Buffer access may be to a negative index in the buffer."
+   )
+ select ba, message, bufferSource, "buffer"

c/cert/src/rules/ARR30-C/DoNotFormOutOfBoundsPointersOrArraySubscripts.ql

@@ -0,0 +1,25 @@
+/**
+ * @id c/cert/library-function-argument-out-of-bounds
+ * @name ARR38-C: Guarantee that library functions do not form invalid pointers
+ * @description Passing out-of-bounds pointers or erroneous size arguments to standard library
+ *              functions can result in out-of-bounds accesses and other undefined behavior.
+ * @kind problem
+ * @precision high
+ * @problem.severity error
+ * @tags external/cert/id/arr38-c
+ *       correctness
+ *       security
+ *       external/cert/obligation/rule
+ */
+
+import cpp
+import codingstandards.c.cert
+import codingstandards.c.OutOfBounds
+
+from
+  OOB::BufferAccessLibraryFunctionCall fc, string message, Expr bufferArg, string bufferArgStr,
+  Expr sizeOrOtherBufferArg, string otherStr
+where
+  not isExcluded(fc, OutOfBoundsPackage::libraryFunctionArgumentOutOfBoundsQuery()) and
+  OOB::problems(fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr)
+select fc, message, bufferArg, bufferArgStr, sizeOrOtherBufferArg, otherStr

c/cert/src/rules/ARR38-C/LibraryFunctionArgumentOutOfBounds.md

@@ -14,14 +14,26 @@
 import cpp
 import codingstandards.c.cert
 
-class ExitFunction extends Function {
-  ExitFunction() { this.hasGlobalName(["_Exit", "exit", "quick_exit", "longjmp"]) }
+/**
+ * Exit function or macro.
+ */
+class Exit extends Locatable {
+  Exit() {
+    ["_Exit", "exit", "quick_exit", "longjmp"] = [this.(Function).getName(), this.(Macro).getName()]
+  }
 }
 
-class ExitFunctionCall extends FunctionCall {
-  ExitFunctionCall() { this.getTarget() instanceof ExitFunction }
+class ExitExpr extends Expr {
+  ExitExpr() {
+    this.(FunctionCall).getTarget() instanceof Exit
+    or
+    any(MacroInvocation m | this = m.getExpr()).getMacro() instanceof Exit
+  }
 }
 
+/**
+ * Functions that are registered as exit handlers.
+ */
 class RegisteredAtexit extends FunctionAccess {
   RegisteredAtexit() {
     exists(FunctionCall ae |
@@ -32,24 +44,26 @@ class RegisteredAtexit extends FunctionAccess {
 }
 
 /**
- * Nodes of type Function, FunctionCall or FunctionAccess that \
- * are reachable from a redistered atexit handler and
+ * Nodes of type Function, FunctionCall, FunctionAccess or ExitExpr
+ * that are reachable from a registered atexit handler and
  * can reach an exit function.
  */
 class InterestingNode extends ControlFlowNode {
   InterestingNode() {
     exists(Function f |
       (
         this = f and
-        // exit functions are not part of edges
-        not this = any(ExitFunction ec)
+        // exit is not part of edges
+        not this instanceof Exit
         or
         this.(FunctionCall).getTarget() = f
         or
         this.(FunctionAccess).getTarget() = f
+        or
+        this.(ExitExpr).getEnclosingFunction() = f
       ) and
-      // reaches an exit function
-      f.calls*(any(ExitFunction e)) and
+      // reaches an `ExitExpr`
+      f.calls*(any(ExitExpr ee).getEnclosingFunction()) and
       // is reachable from a registered atexit function
       exists(RegisteredAtexit re | re.getTarget().calls*(f))
     )
@@ -62,14 +76,12 @@ class InterestingNode extends ControlFlowNode {
  * `Function` and `FunctionCall` in their body.
  */
 query predicate edges(InterestingNode a, InterestingNode b) {
-  a.(FunctionAccess).getTarget() = b
-  or
-  a.(FunctionCall).getTarget() = b
-  or
+  a.(FunctionAccess).getTarget() = b or
+  a.(FunctionCall).getTarget() = b or
   a.(Function).calls(_, b)
 }
 
-from RegisteredAtexit hr, Function f, ExitFunctionCall e
+from RegisteredAtexit hr, Function f, ExitExpr e
 where edges(hr, f) and edges+(f, e)
 select f, hr, e, "The function is $@ and $@. It must instead terminate by returning.", hr,
   "registered as `exit handler`", e, "calls an `exit function`"

c/cert/src/rules/ARR38-C/LibraryFunctionArgumentOutOfBounds.ql

@@ -441,7 +441,7 @@ ControlFlowNode ferrorNotchecked(FileWriteFunctionCall write) {
     not isShortCircuitedEdge(mid, result) and
     result = mid.getASuccessor() and
     //Stop recursion on call to ferror on the correct file
-    not accessSameTarget(result.(FerrorCall).getArgument(0), write.getFileExpr())
+    not sameFileSource(result.(FerrorCall), write)
   )
 }