Merge branch 'main' into cwe497b

cpp/ql/lib/DefaultOptions.qll

@@ -54,11 +54,13 @@ class Options extends string {
    *
    * By default, this holds for `exit`, `_exit`, `abort`, `__assert_fail`,
    * `longjmp`, `__builtin_unreachable` and any function with a
-   * `noreturn` attribute.
+   * `noreturn` attribute or specifier.
    */
   predicate exits(Function f) {
     f.getAnAttribute().hasName("noreturn")
     or
+    f.getASpecifier().hasName("noreturn")
+    or
     f.hasGlobalOrStdName([
         "exit", "_exit", "abort", "__assert_fail", "longjmp", "__builtin_unreachable"
       ])

cpp/ql/lib/Options.qll

@@ -39,7 +39,7 @@ class CustomOptions extends Options {
    *
    * By default, this holds for `exit`, `_exit`, `abort`, `__assert_fail`,
    * `longjmp`, `error`, `__builtin_unreachable` and any function with a
-   * `noreturn` attribute.
+   * `noreturn` attribute or specifier.
    */
   override predicate exits(Function f) { Options.super.exits(f) }
 

cpp/ql/lib/change-notes/2022-03-10-template-implicit-copy.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* `hasImplicitCopyConstructor` and `hasImplicitCopyAssignmentOperator` now correctly handle implicitly-deleted operators in templates.

cpp/ql/lib/change-notes/2022-03-14-c11-noreturn.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* `DefaultOptions::exits` now holds for C11 functions with the `_Noreturn` or `noreturn` specifier.

cpp/ql/lib/semmle/code/cpp/Class.qll

@@ -251,6 +251,16 @@ class Class extends UserType {
     not this.implicitCopyConstructorDeleted() and
     forall(CopyConstructor cc | cc = this.getAMemberFunction() |
       cc.isCompilerGenerated() and not cc.isDeleted()
+    ) and
+    (
+      not this instanceof ClassTemplateInstantiation
+      or
+      this.(ClassTemplateInstantiation).getTemplate().hasImplicitCopyConstructor()
+    ) and
+    (
+      not this instanceof PartialClassTemplateSpecialization
+      or
+      this.(PartialClassTemplateSpecialization).getPrimaryTemplate().hasImplicitCopyConstructor()
     )
   }
 
@@ -266,6 +276,18 @@ class Class extends UserType {
     not this.implicitCopyAssignmentOperatorDeleted() and
     forall(CopyAssignmentOperator ca | ca = this.getAMemberFunction() |
       ca.isCompilerGenerated() and not ca.isDeleted()
+    ) and
+    (
+      not this instanceof ClassTemplateInstantiation
+      or
+      this.(ClassTemplateInstantiation).getTemplate().hasImplicitCopyAssignmentOperator()
+    ) and
+    (
+      not this instanceof PartialClassTemplateSpecialization
+      or
+      this.(PartialClassTemplateSpecialization)
+          .getPrimaryTemplate()
+          .hasImplicitCopyAssignmentOperator()
     )
   }
 

cpp/ql/lib/semmle/code/cpp/Specifier.qll

@@ -38,7 +38,7 @@ class FunctionSpecifier extends Specifier {
 
 /**
  * A C/C++ storage class specifier: `auto`, `register`, `static`, `extern`,
- * or `mutable".
+ * or `mutable`.
  */
 class StorageClassSpecifier extends Specifier {
   StorageClassSpecifier() { this.hasName(["auto", "register", "static", "extern", "mutable"]) }

cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll

@@ -447,26 +447,6 @@ private predicate skipInitializer(Initializer init) {
   )
 }
 
-/**
- * Holds if `e` is an expression in a static initializer that must be evaluated
- * at run time. This predicate computes "is non-const" instead of "is const" in
- * order to avoid recursion through forall.
- */
-private predicate runtimeExprInStaticInitializer(Expr e) {
-  inStaticInitializer(e) and
-  if e instanceof AggregateLiteral
-  then runtimeExprInStaticInitializer(e.getAChild())
-  else not e.getFullyConverted().isConstant()
-}
-
-/** Holds if `e` is part of the initializer of a local static variable. */
-private predicate inStaticInitializer(Expr e) {
-  exists(LocalVariable local |
-    local.isStatic() and
-    e.getParent+() = local.getInitializer()
-  )
-}
-
 /**
  * Gets the `i`th child of `n` in control-flow order, where the `i`-indexes are
  * contiguous, and the first index is 0.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll

@@ -287,20 +287,6 @@ private module SsaDefReaches {
     )
   }
 
-  /**
-   * Holds if the SSA definition of `v` at `def` reaches uncertain SSA definition
-   * `redef` in the same basic block, without crossing another SSA definition of `v`.
-   */
-  predicate ssaDefReachesUncertainDefWithinBlock(
-    SourceVariable v, Definition def, UncertainWriteDefinition redef
-  ) {
-    exists(BasicBlock bb, int rnk, int i |
-      ssaDefReachesRank(bb, def, rnk, v) and
-      rnk = ssaRefRank(bb, i, v, SsaDef()) - 1 and
-      redef.definesAt(v, bb, i)
-    )
-  }
-
   /**
    * Same as `ssaRefRank()`, but restricted to a particular SSA definition `def`.
    */

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/reachability/DominanceInternal.qll

@@ -1,7 +1,5 @@
 private import ReachableBlock as Reachability
 
-private module ReachabilityGraph = Reachability::Graph;
-
 module Graph {
   import Reachability::Graph
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/reachability/DominanceInternal.qll

@@ -1,7 +1,5 @@
 private import ReachableBlock as Reachability
 
-private module ReachabilityGraph = Reachability::Graph;
-
 module Graph {
   import Reachability::Graph
 

cpp/ql/src/experimental/Security/CWE/CWE-377/InsecureTemporaryFile.ql

@@ -13,30 +13,6 @@
 import cpp
 import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 
-/** Holds for a function `f` that has an argument at index `apos` used to read the file. */
-predicate numberArgumentRead(Function f, int apos) {
-  f.hasGlobalOrStdName("fgets") and apos = 2
-  or
-  f.hasGlobalOrStdName("fread") and apos = 3
-  or
-  f.hasGlobalOrStdName("read") and apos = 0
-  or
-  f.hasGlobalOrStdName("fscanf") and apos = 0
-}
-
-/** Holds for a function `f` that has an argument at index `apos` used to write to file */
-predicate numberArgumentWrite(Function f, int apos) {
-  f.hasGlobalOrStdName("fprintf") and apos = 0
-  or
-  f.hasGlobalOrStdName("fputs") and apos = 1
-  or
-  f.hasGlobalOrStdName("write") and apos = 0
-  or
-  f.hasGlobalOrStdName("fwrite") and apos = 3
-  or
-  f.hasGlobalOrStdName("fflush") and apos = 0
-}
-
 from FunctionCall fc, string msg
 where
   // search for functions for generating a name, without a guarantee of the absence of a file during the period of work with it.
@@ -59,54 +35,4 @@ where
   ) and
   msg =
     "Finding the name of a file that does not exist does not mean that it will not be exist at the next operation."
-  or
-  // finding places to work with a file without setting permissions, but with predictable names.
-  (
-    fc.getTarget().hasGlobalOrStdName("fopen") or
-    fc.getTarget().hasGlobalOrStdName("open")
-  ) and
-  fc.getNumberOfArguments() = 2 and
-  exists(FunctionCall fctmp, int i |
-    numberArgumentWrite(fctmp.getTarget(), i) and
-    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
-  ) and
-  not exists(FunctionCall fctmp, int i |
-    numberArgumentRead(fctmp.getTarget(), i) and
-    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
-  ) and
-  exists(FunctionCall fctmp |
-    (
-      fctmp.getTarget().hasGlobalOrStdName("strcat") or
-      fctmp.getTarget().hasGlobalOrStdName("strcpy")
-    ) and
-    globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp.getAnArgument())
-    or
-    fctmp.getTarget().hasGlobalOrStdName("getenv") and
-    globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp)
-    or
-    (
-      fctmp.getTarget().hasGlobalOrStdName("asprintf") or
-      fctmp.getTarget().hasGlobalOrStdName("vasprintf") or
-      fctmp.getTarget().hasGlobalOrStdName("xasprintf") or
-      fctmp.getTarget().hasGlobalOrStdName("xvasprintf ")
-    ) and
-    exists(Variable vrtmp |
-      vrtmp = fc.getArgument(0).(VariableAccess).getTarget() and
-      vrtmp = fctmp.getArgument(0).(AddressOfExpr).getAddressable() and
-      not vrtmp instanceof Field
-    )
-  ) and
-  not exists(FunctionCall fctmp |
-    (
-      fctmp.getTarget().hasGlobalOrStdName("umask") or
-      fctmp.getTarget().hasGlobalOrStdName("fchmod") or
-      fctmp.getTarget().hasGlobalOrStdName("chmod")
-    ) and
-    (
-      fc.getBasicBlock().getASuccessor*() = fctmp.getBasicBlock() or
-      fctmp.getBasicBlock().getASuccessor*() = fc.getBasicBlock()
-    )
-  ) and
-  msg =
-    "Creating a file for writing without evaluating its existence and setting permissions can be unsafe."
 select fc, msg

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/InsecureTemporaryFile.expected

@@ -1,2 +1 @@
 | test.cpp:16:20:16:25 | call to tmpnam | Finding the name of a file that does not exist does not mean that it will not be exist at the next operation. |
-| test.cpp:42:8:42:12 | call to fopen | Creating a file for writing without evaluating its existence and setting permissions can be unsafe. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-377/semmle/tests/test.cpp

@@ -39,7 +39,7 @@ int funcTest3()
   FILE *fp;
   char filename[80];
   strcat(filename, "/tmp/tmp.name");
-  fp = fopen(filename,"w"); // BAD
+  fp = fopen(filename,"w"); // BAD [NOT DETECTED]
   fprintf(fp,"%s\n","data to file");
   fclose(fp);
   return 0;

cpp/ql/test/library-tests/special_members/generated_copy/assign.expected

@@ -11,6 +11,7 @@
 | difference::Base | can | does NOT | have implicit copy assignment |
 | difference::OnlyAssign | can | does | have implicit copy assignment |
 | difference::OnlyCtor | can NOT | does NOT | have implicit copy assignment |
+| instantiated_explicit_ctor::Wrapper<int> | can | does | have implicit copy assignment |
 | moves::MoveAssign | can NOT | does NOT | have implicit copy assignment |
 | moves::MoveCtor | can NOT | does NOT | have implicit copy assignment |
 | private_cc::C | can | does NOT | have implicit copy assignment |

cpp/ql/test/library-tests/special_members/generated_copy/copy.cpp

@@ -131,3 +131,21 @@ namespace difference {
   class OnlyAssign : Base {
   };
 }
+
+namespace instantiated_explicit_ctor {
+  template<class T>
+  class Wrapper {
+  public:
+    Wrapper(Wrapper<T> &other) {
+      m_t = other.m_t;
+    }
+    
+    Wrapper() {
+      m_t = 0;
+    }
+  private:
+    T m_t;
+  };
+
+  Wrapper<int> wrapped_int;
+}

cpp/ql/test/library-tests/special_members/generated_copy/ctor.expected

@@ -11,6 +11,7 @@
 | difference::Base | can | does NOT | have implicit copy constructor |
 | difference::OnlyAssign | can NOT | does NOT | have implicit copy constructor |
 | difference::OnlyCtor | can | does | have implicit copy constructor |
+| instantiated_explicit_ctor::Wrapper<int> | can | does NOT | have implicit copy constructor |
 | moves::MoveAssign | can NOT | does NOT | have implicit copy constructor |
 | moves::MoveCtor | can NOT | does NOT | have implicit copy constructor |
 | private_cc::C | can | does NOT | have implicit copy constructor |

cpp/ql/test/library-tests/special_members/generated_copy/functions.expected

@@ -86,5 +86,9 @@
 | copy.cpp:131:9:131:9 | OnlyAssign | deleted |  |
 | copy.cpp:131:9:131:9 | operator= |  |  |
 | copy.cpp:131:9:131:9 | operator= |  |  |
+| copy.cpp:137:9:137:9 | operator= |  |  |
+| copy.cpp:139:5:139:11 | Wrapper |  |  |
+| copy.cpp:143:5:143:5 | Wrapper |  |  |
+| copy.cpp:143:5:143:11 | Wrapper |  |  |
 | file://:0:0:0:0 | operator= |  |  |
 | file://:0:0:0:0 | operator= |  |  |

cpp/ql/test/query-tests/jsf/4.13 Functions/AV Rule 114/test.c

@@ -1,4 +1,4 @@
-
+// semmle-extractor-options: -std=c11
 int f1(void) {
     int x = 1;
     return 2;
@@ -99,3 +99,9 @@ int f14()
 {
 	__asm__("rdtsc"); // GOOD
 }
+
+_Noreturn void f15();
+
+int f16() {
+    f15(); // GOOD
+}

cpp/ql/test/query-tests/jsf/4.13 Functions/AV Rule 114/test.cpp

@@ -1,4 +1,4 @@
-
+// semmle-extractor-options: -std=c++11
 class MyValue {
 public:
 	MyValue(int _val) : val(_val) {};
@@ -164,3 +164,9 @@ int g19(int x)
 
 	return x; // GOOD
 }
+
+[[noreturn]] void g20();
+
+int g21() {
+    g20(); // GOOD
+}