Merge remote-tracking branch 'origin/main' into michaelrfairhurst/implement-misra-amd4-rule-amendments

Author: MichaelRFairhurst

.github/workflows/code-scanning-pack-gen.yml

@@ -80,6 +80,8 @@ jobs:
 
       - name: Checkout external help files
         id: checkout-external-help-files
+        # PRs from forks and dependabot do not have access to an appropriate token for cloning the help files repos
+        if: ${{ !github.event.pull_request.head.repo.fork && github.actor != 'dependabot[bot]' }}
         uses: actions/checkout@v4
         with:
           ssh-key: ${{ secrets.CODEQL_CODING_STANDARDS_HELP_KEY }}
@@ -88,7 +90,7 @@ jobs:
           path: external-help-files
 
       - name: Include external help files
-        if: steps.checkout-external-help-files.outcome == 'success'
+        if: ${{ !github.event.pull_request.head.repo.fork && github.actor != 'dependabot[bot]'&& steps.checkout-external-help-files.outcome == 'success' }}
         run: |
           pushd external-help-files
           find . -name '*.md' -exec rsync -av --relative {} "$GITHUB_WORKSPACE" \;

.github/workflows/upgrade_codeql_dependencies.yml

@@ -53,7 +53,7 @@ jobs:
           find c \( -name '*.ql' -or -name '*.qll' \) -print0 | xargs -0 --max-procs "$XARGS_MAX_PROCS" codeql query format --in-place
 
       - name: Create Pull Request
-        uses: peter-evans/create-pull-request@5e914681df9dc83aa4e4905692ca88beb2f9e91f # v7.0.5
+        uses: peter-evans/create-pull-request@67ccf781d68cd99b580ae25a5c18a1cc84ffff1f # v7.0.6
         with:
           title: "Upgrade `github/codeql` dependency to ${{ github.event.inputs.codeql_cli_version }}"
           body: |

amendments.csv

@@ -1,49 +1,50 @@
 language,standard,amendment,rule_id,supportable,implementation_category,implemented,difficulty
-c,MISRA-C-2012,Amendment3,DIR-4-6,Yes,Expand,No,Easy
+c,MISRA-C-2012,Amendment3,DIR-4-6,Yes,Expand,Yes,Easy
 c,MISRA-C-2012,Amendment3,DIR-4-9,Yes,Refine,No,Easy
 c,MISRA-C-2012,Amendment3,DIR-4-11,Yes,Refine,No,Import
 c,MISRA-C-2012,Amendment3,RULE-1-4,Yes,Replace,No,Easy
-c,MISRA-C-2012,Amendment3,RULE-10-1,Yes,Replace,No,Easy
-c,MISRA-C-2012,Amendment3,RULE-10-3,Yes,Refine,No,Easy
-c,MISRA-C-2012,Amendment3,RULE-10-4,Yes,Refine,No,Import
-c,MISRA-C-2012,Amendment3,RULE-10-5,Yes,Expand,No,Easy
-c,MISRA-C-2012,Amendment3,RULE-10-7,Yes,Refine,No,Import
-c,MISRA-C-2012,Amendment3,RULE-10-8,Yes,Refine,No,Import
-c,MISRA-C-2012,Amendment3,RULE-21-11,Yes,Clarification,No,Import
+c,MISRA-C-2012,Amendment3,RULE-10-1,Yes,Replace,Yes,Easy
+c,MISRA-C-2012,Amendment3,RULE-10-3,Yes,Refine,Yes,Easy
+c,MISRA-C-2012,Amendment3,RULE-10-4,Yes,Refine,Yes,Import
+c,MISRA-C-2012,Amendment3,RULE-10-5,Yes,Expand,Yes,Easy
+c,MISRA-C-2012,Amendment3,RULE-10-7,Yes,Refine,Yes,Import
+c,MISRA-C-2012,Amendment3,RULE-10-8,Yes,Refine,Yes,Import
+c,MISRA-C-2012,Amendment3,RULE-21-11,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Amendment3,RULE-21-12,Yes,Replace,No,Easy
 c,MISRA-C-2012,Amendment4,RULE-11-3,Yes,Expand,Yes,Easy
 c,MISRA-C-2012,Amendment4,RULE-11-8,Yes,Expand,Yes,Easy
 c,MISRA-C-2012,Amendment4,RULE-13-2,Yes,Expand,Yes,Very Hard
 c,MISRA-C-2012,Amendment4,RULE-18-6,Yes,Expand,No,Medium
 c,MISRA-C-2012,Amendment4,RULE-18-8,Yes,Split,Yes,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-2-2,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-2-7,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-3-1,Yes,Refine,Yes,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-8-6,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-8-9,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-9-4,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-10-1,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-18-3,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-1-4,Yes,Replace,No,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-9-1,Yes,Refine,Yes,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-9-2,Yes,Refine,No,Import
-c,MISRA-C-2012,Corrigendum2,DIR-4-10,Yes,Clarification,No,Import
+c,MISRA-C-2012,Amendment4,RULE-2-2,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Amendment4,RULE-2-7,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Amendment4,RULE-3-1,Yes,Refine,Yes,Easy
+c,MISRA-C-2012,Amendment4,RULE-8-6,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Amendment4,RULE-8-9,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Amendment4,RULE-9-4,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Amendment4,RULE-10-1,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Amendment4,RULE-18-3,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Amendment4,RULE-1-4,Yes,Replace,No,Easy
+c,MISRA-C-2012,Amendment4,RULE-9-1,Yes,Refine,Yes,Easy
+c,MISRA-C-2012,Amendment4,RULE-9-2,Yes,Refine,No,Import
+c,MISRA-C-2012,Corrigendum2,DIR-4-10,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Corrigendum2,RULE-7-4,Yes,Refine,No,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-8-2,Yes,Clarification,No,Import
+c,MISRA-C-2012,Corrigendum2,RULE-8-2,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Corrigendum2,RULE-8-3,Yes,Refine,No,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-8-7,Yes,Clarification,No,Import
+c,MISRA-C-2012,Corrigendum2,RULE-8-7,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-10-1,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Corrigendum2,RULE-10-2,Yes,Refine,No,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-10-3,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-11-3,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-11-6,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-13-2,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-13-6,Yes,Clarification,No,Import
+c,MISRA-C-2012,Corrigendum2,RULE-10-3,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-11-3,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-11-6,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-13-2,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-13-6,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Corrigendum2,RULE-14-3,Yes,Refine,No,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-15-7,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-17-4,Yes,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-17-5,Yes,Clarification,No,Import
+c,MISRA-C-2012,Corrigendum2,RULE-15-7,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-17-4,Yes,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-17-5,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Corrigendum2,RULE-18-1,Yes,Refine,No,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-20-14,No,Clarification,No,Import
-c,MISRA-C-2012,Corrigendum2,RULE-21-19,Yes,Clarification,No,Import
+c,MISRA-C-2012,Corrigendum2,RULE-20-14,No,Clarification,Yes,Import
+c,MISRA-C-2012,Corrigendum2,RULE-21-19,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Corrigendum2,RULE-21-20,Yes,Refine,No,Easy
-c,MISRA-C-2012,Corrigendum2,RULE-22-9,Yes,Clarification,No,Import
+c,MISRA-C-2012,Corrigendum2,RULE-22-9,Yes,Clarification,Yes,Import

c/cert/src/codeql-pack.lock.yml

@@ -2,23 +2,23 @@
 lockVersion: 1.0.0
 dependencies:
   codeql/cpp-all:
-    version: 1.4.2
+    version: 2.1.1
   codeql/dataflow:
-    version: 1.1.1
+    version: 1.1.6
   codeql/mad:
-    version: 1.0.7
+    version: 1.0.12
   codeql/rangeanalysis:
-    version: 1.0.7
+    version: 1.0.12
   codeql/ssa:
-    version: 1.0.7
+    version: 1.0.12
   codeql/tutorial:
-    version: 1.0.7
+    version: 1.0.12
   codeql/typeflow:
-    version: 1.0.7
+    version: 1.0.12
   codeql/typetracking:
-    version: 1.0.7
+    version: 1.0.12
   codeql/util:
-    version: 1.0.7
+    version: 1.0.12
   codeql/xml:
-    version: 1.0.7
+    version: 1.0.12
 compiled: false

c/cert/src/qlpack.yml

@@ -1,8 +1,8 @@
 name: codeql/cert-c-coding-standards
-version: 2.40.0-dev
+version: 2.43.0-dev
 description: CERT C 2016
 suites: codeql-suites
 license: MIT
 dependencies:
   codeql/common-c-coding-standards: '*'
-  codeql/cpp-all: 1.4.2
+  codeql/cpp-all: 2.1.1

c/cert/src/rules/CON30-C/CleanUpThreadSpecificStorage.ql

@@ -15,7 +15,6 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.cpp.Concurrency
-import semmle.code.cpp.dataflow.TaintTracking
 import semmle.code.cpp.dataflow.DataFlow
 
 module TssCreateToTssDeleteConfig implements DataFlow::ConfigSig {

c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql

@@ -14,30 +14,43 @@
 
 import cpp
 import codingstandards.c.cert
+import codingstandards.c.Objects
 import codingstandards.cpp.Concurrency
-import semmle.code.cpp.dataflow.TaintTracking
 import semmle.code.cpp.dataflow.DataFlow
 import semmle.code.cpp.commons.Alloc
 
-from C11ThreadCreateCall tcc, StackVariable sv, Expr arg, Expr acc
+from C11ThreadCreateCall tcc, Expr arg
 where
   not isExcluded(tcc, Concurrency4Package::appropriateThreadObjectStorageDurationsQuery()) and
   tcc.getArgument(2) = arg and
-  sv.getAnAccess() = acc and
-  // a stack variable that is given as an argument to a thread
-  TaintTracking::localTaint(DataFlow::exprNode(acc), DataFlow::exprNode(arg)) and
-  // or isn't one of the allowed usage patterns
-  not exists(Expr mfc |
-    isAllocationExpr(mfc) and
-    sv.getAnAssignedValue() = mfc and
-    acc.getAPredecessor*() = mfc
-  ) and
-  not exists(TSSGetFunctionCall tsg, TSSSetFunctionCall tss, DataFlow::Node src |
-    sv.getAnAssignedValue() = tsg and
-    acc.getAPredecessor*() = tsg and
-    // there should be dataflow from somewhere (the same somewhere)
-    // into each of the first arguments
-    DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
-    DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
+  (
+    exists(ObjectIdentity obj, Expr acc |
+      obj.getASubobjectAccess() = acc and
+      obj.getStorageDuration().isAutomatic() and
+      exists(DataFlow::Node addrNode |
+        (
+          addrNode = DataFlow::exprNode(any(AddressOfExpr e | e.getOperand() = acc))
+          or
+          addrNode = DataFlow::exprNode(acc) and
+          exists(ArrayToPointerConversion c | c.getExpr() = acc)
+        ) and
+        TaintTracking::localTaint(addrNode, DataFlow::exprNode(arg))
+      )
+    )
+    or
+    // TODO: This case is handling threadlocals in a useful way that's not intended to be covered
+    // by the rule. See issue #801. The actual rule should expect no tss_t objects is used, and
+    // this check that this is initialized doesn't seem to belong here. However, it is a useful
+    // check in and of itself, so we should figure out if this is part of an optional rule we
+    // haven't yet implemented and move this behavior there.
+    exists(TSSGetFunctionCall tsg |
+      TaintTracking::localTaint(DataFlow::exprNode(tsg), DataFlow::exprNode(arg)) and
+      not exists(TSSSetFunctionCall tss, DataFlow::Node src |
+        // there should be dataflow from somewhere (the same somewhere)
+        // into each of the first arguments
+        DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
+        DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
+      )
+    )
   )
 select tcc, "$@ not declared with appropriate storage duration", arg, "Shared object"

c/cert/src/rules/CON34-C/ThreadObjectStorageDurationsNotInitialized.ql

@@ -16,7 +16,6 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.cpp.Concurrency
-import semmle.code.cpp.dataflow.TaintTracking
 import semmle.code.cpp.dataflow.DataFlow
 
 from TSSGetFunctionCall tsg, ThreadedFunction tf

c/cert/src/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.ql

@@ -14,37 +14,10 @@
 
 import cpp
 import codingstandards.c.cert
-import codingstandards.cpp.Concurrency
+import codingstandards.cpp.rules.joinordetachthreadonlyonce.JoinOrDetachThreadOnlyOnce
 
-// OK
-// 1) Thread calls detach parent DOES NOT call join
-// 2) Parent calls join, thread does NOT call detach()
-// NOT OK
-// 1) Thread calls detach, parent calls join
-// 2) Thread calls detach twice, parent does not call join
-// 3) Parent calls join twice, thread does not call detach
-from C11ThreadCreateCall tcc
-where
-  not isExcluded(tcc, Concurrency5Package::threadWasPreviouslyJoinedOrDetachedQuery()) and
-  // Note: These cases can be simplified but they are presented like this for clarity
-  // case 1 - calls to `thrd_join` and `thrd_detach` within the parent or
-  // within the parent / child CFG.
-  exists(C11ThreadWait tw, C11ThreadDetach dt |
-    tw = getAThreadContextAwareSuccessor(tcc) and
-    dt = getAThreadContextAwareSuccessor(tcc)
-  )
-  or
-  // case 2 - multiple calls to `thrd_detach` within the threaded CFG.
-  exists(C11ThreadDetach dt1, C11ThreadDetach dt2 |
-    dt1 = getAThreadContextAwareSuccessor(tcc) and
-    dt2 = getAThreadContextAwareSuccessor(tcc) and
-    not dt1 = dt2
-  )
-  or
-  // case 3 - multiple calls to `thrd_join` within the threaded CFG.
-  exists(C11ThreadWait tw1, C11ThreadWait tw2 |
-    tw1 = getAThreadContextAwareSuccessor(tcc) and
-    tw2 = getAThreadContextAwareSuccessor(tcc) and
-    not tw1 = tw2
-  )
-select tcc, "Thread may call join or detach after the thread is joined or detached."
+class ThreadWasPreviouslyJoinedOrDetachedQuery extends JoinOrDetachThreadOnlyOnceSharedQuery {
+  ThreadWasPreviouslyJoinedOrDetachedQuery() {
+    this = Concurrency5Package::threadWasPreviouslyJoinedOrDetachedQuery()
+  }
+}

c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql

@@ -13,10 +13,16 @@
 
 import cpp
 import codingstandards.c.cert
+import codingstandards.c.Objects
 import semmle.code.cpp.dataflow.DataFlow
 
-class Source extends StackVariable {
-  Source() { not this instanceof Parameter }
+class Source extends Expr {
+  ObjectIdentity rootObject;
+
+  Source() {
+    rootObject.getStorageDuration().isAutomatic() and
+    this = rootObject.getASubobjectAddressExpr()
+  }
 }
 
 class Sink extends DataFlow::Node {
@@ -40,7 +46,7 @@ from DataFlow::Node src, DataFlow::Node sink
 where
   not isExcluded(sink.asExpr(),
     Declarations8Package::appropriateStorageDurationsFunctionReturnQuery()) and
-  exists(Source s | src.asExpr() = s.getAnAccess()) and
+  exists(Source s | src.asExpr() = s) and
   sink instanceof Sink and
   DataFlow::localFlow(src, sink)
 select sink, "$@ with automatic storage may be accessible outside of its lifetime.", src,

c/cert/src/rules/ERR30-C/ErrnoReadBeforeReturn.ql

@@ -14,6 +14,7 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.c.Errno
+import semmle.code.cpp.dataflow.DataFlow
 
 /**
  * A call to an `OutOfBandErrnoSettingFunction`

c/cert/src/rules/ERR30-C/FunctionCallBeforeErrnoCheck.ql

@@ -14,7 +14,6 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.c.Errno
-import semmle.code.cpp.dataflow.DataFlow
 
 /**
  * A call to an `OutOfBandErrnoSettingFunction`

c/cert/src/rules/ERR30-C/SetlocaleMightSetErrno.ql

@@ -13,6 +13,7 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.c.Errno
+import semmle.code.cpp.dataflow.DataFlow
 
 class SetlocaleFunctionCall extends FunctionCall {
   SetlocaleFunctionCall() { this.getTarget().hasGlobalName("setlocale") }

c/cert/src/rules/ERR32-C/DoNotRelyOnIndeterminateValuesOfErrno.ql

@@ -20,17 +20,17 @@ import semmle.code.cpp.controlflow.Guards
  * A check on `signal` call return value
  * `if (signal(SIGINT, handler) == SIG_ERR)`
  */
-class SignalCheckOperation extends EqualityOperation, GuardCondition {
+class SignalCheckOperation extends EqualityOperation instanceof GuardCondition {
   BasicBlock errorSuccessor;
 
   SignalCheckOperation() {
     this.getAnOperand() = any(MacroInvocation m | m.getMacroName() = "SIG_ERR").getExpr() and
     (
       this.getOperator() = "==" and
-      this.controls(errorSuccessor, true)
+      super.controls(errorSuccessor, true)
       or
       this.getOperator() = "!=" and
-      this.controls(errorSuccessor, false)
+      super.controls(errorSuccessor, false)
     )
   }
 

c/cert/src/rules/EXP30-C/DependenceOnOrderOfFunctionArgumentsForSideEffects.ql

@@ -14,7 +14,6 @@
 import cpp
 import codingstandards.c.cert
 import codingstandards.cpp.SideEffect
-import semmle.code.cpp.dataflow.DataFlow
 import semmle.code.cpp.dataflow.TaintTracking
 import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 

c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql

@@ -13,15 +13,13 @@
 
 import cpp
 import codingstandards.c.cert
-import codingstandards.cpp.lifetimes.CLifetimes
+import codingstandards.c.Objects
 
 // Note: Undefined behavior is possible regardless of whether the accessed field from the returned
 // struct is an array or a scalar (i.e. arithmetic and pointer types) member, according to the standard.
-from FieldAccess fa, FunctionCall fc
+from FieldAccess fa, TemporaryObjectIdentity tempObject
 where
   not isExcluded(fa, InvalidMemory2Package::doNotModifyObjectsWithTemporaryLifetimeQuery()) and
-  not fa.getQualifier().isLValue() and
-  fa.getQualifier().getUnconverted() = fc and
-  fa.getQualifier().getUnconverted().getUnspecifiedType() instanceof StructOrUnionTypeWithArrayField
-select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", fc,
-  "function call"
+  fa.getQualifier().getUnconverted() = tempObject
+select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", tempObject,
+  "temporary object"

c/cert/src/rules/FIO37-C/SuccessfulFgetsOrFgetwsMayReturnAnEmptyString.ql

@@ -14,7 +14,7 @@ import cpp
 import codingstandards.c.cert
 import codingstandards.cpp.FgetsErrorManagement
 import codingstandards.cpp.Dereferenced
-import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.dataflow.DataFlow
 
 /*
  * CFG nodes that follows a successful call to `fgets`

c/cert/src/rules/FLP32-C/UncheckedRangeDomainPoleErrors.md

@@ -345,7 +345,7 @@ Independent( INT34-C, FLP32-C, INT33-C) CWE-682 = Union( FLP32-C, list) where li
 
 ## Implementation notes
 
-None
+This query identifies possible domain, pole and range errors on a selection of C standard library fuctions from math.h.
 
 ## References