Merge branch 'main' into jorgectf/python/ldapinsecureauth

Author: RasmusWL

.github/workflows/csv-coverage-update.yml

@@ -8,7 +8,7 @@ on:
 jobs:
   update:
     name: Update framework coverage report
-    if: github.event.repository.fork == false
+    if: github.repository == 'github/codeql'
     runs-on: ubuntu-latest
 
     steps:

cpp/change-notes/2021-06-22-sql-tainted.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The 'Uncontrolled data in SQL query' (cpp/sql-injection) query now supports the `libpqxx` library.

cpp/change-notes/2021-09-13-overflow-static.md

@@ -0,0 +1,4 @@
+lgtm,codescanning
+* The `memberMayBeVarSize` predicate considers more fields to be variable size.
+  As a result, the "Static buffer overflow" query (cpp/static-buffer-overflow)
+  produces fewer false positives.

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

@@ -171,7 +171,7 @@ class Container extends Locatable, @container {
  * To get the full path, use `getAbsolutePath`.
  */
 class Folder extends Container, @folder {
-  override string getAbsolutePath() { folders(underlyingElement(this), result, _) }
+  override string getAbsolutePath() { folders(underlyingElement(this), result) }
 
   override Location getLocation() {
     result.getContainer() = this and
@@ -190,7 +190,7 @@ class Folder extends Container, @folder {
    * DEPRECATED: use `getAbsolutePath` instead.
    * Gets the name of this folder.
    */
-  deprecated string getName() { folders(underlyingElement(this), result, _) }
+  deprecated string getName() { folders(underlyingElement(this), result) }
 
   /**
    * DEPRECATED: use `getAbsolutePath` instead.
@@ -208,17 +208,7 @@ class Folder extends Container, @folder {
    * DEPRECATED: use `getBaseName` instead.
    * Gets the last part of the folder name.
    */
-  deprecated string getShortName() {
-    exists(string longnameRaw, string longname |
-      folders(underlyingElement(this), _, longnameRaw) and
-      longname = longnameRaw.replaceAll("\\", "/")
-    |
-      exists(int index |
-        result = longname.splitAt("/", index) and
-        not exists(longname.splitAt("/", index + 1))
-      )
-    )
-  }
+  deprecated string getShortName() { result = this.getBaseName() }
 
   /**
    * DEPRECATED: use `getParentContainer` instead.
@@ -242,7 +232,7 @@ class Folder extends Container, @folder {
  * `getStem` and `getExtension`. To get the full path, use `getAbsolutePath`.
  */
 class File extends Container, @file {
-  override string getAbsolutePath() { files(underlyingElement(this), result, _, _, _) }
+  override string getAbsolutePath() { files(underlyingElement(this), result) }
 
   override string toString() { result = Container.super.toString() }
 
@@ -336,7 +326,13 @@ class File extends Container, @file {
    * for example, for "file.tar.gz", this predicate will have the result
    * "tar.gz", while `getExtension` will have the result "gz".
    */
-  string getExtensions() { files(underlyingElement(this), _, _, result, _) }
+  string getExtensions() {
+    exists(string name, int firstDotPos |
+      name = this.getBaseName() and
+      firstDotPos = min([name.indexOf("."), name.length() - 1]) and
+      result = name.suffix(firstDotPos + 1)
+    )
+  }
 
   /**
    * Gets the short name of this file, that is, the prefix of its base name up
@@ -351,7 +347,16 @@ class File extends Container, @file {
    * for example, for "file.tar.gz", this predicate will have the result
    * "file", while `getStem` will have the result "file.tar".
    */
-  string getShortName() { files(underlyingElement(this), _, result, _, _) }
+  string getShortName() {
+    exists(string name, int firstDotPos |
+      name = this.getBaseName() and
+      firstDotPos = min([name.indexOf("."), name.length()]) and
+      result = name.prefix(firstDotPos)
+    )
+    or
+    this.getAbsolutePath() = "" and
+    result = ""
+  }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll

@@ -2,17 +2,18 @@ import cpp
 import semmle.code.cpp.dataflow.DataFlow
 
 /**
- * Holds if `v` is a member variable of `c` that looks like it might be variable sized in practice.  For
- * example:
+ * Holds if `v` is a member variable of `c` that looks like it might be variable sized
+ * in practice. For example:
  * ```
  * struct myStruct { // c
  *   int amount;
  *   char data[1]; // v
  * };
  * ```
- * This requires that `v` is an array of size 0 or 1, and `v` is the last member of `c`.  In addition,
- * there must be at least one instance where a `c` pointer is allocated with additional space.  For
- * example, holds for `c` if it occurs as
+ * This requires that `v` is an array of size 0 or 1, and `v` is the last member of `c`.
+ * In addition, if the size of the structure is taken, there must be at least one instance
+ * where a `c` pointer is allocated with additional space.
+ * For example, holds for `c` if it occurs as
  * ```
  * malloc(sizeof(c) + 100 * sizeof(char))
  * ```
@@ -27,27 +28,25 @@ predicate memberMayBeVarSize(Class c, MemberVariable v) {
     i = max(int j | c.getCanonicalMember(j) instanceof Field | j) and
     v = c.getCanonicalMember(i) and
     // v is an array of size at most 1
-    v.getUnspecifiedType().(ArrayType).getArraySize() <= 1
+    v.getUnspecifiedType().(ArrayType).getArraySize() <= 1 and
+    not c instanceof Union
   ) and
+  // If the size is taken, then arithmetic is performed on the result at least once
   (
+    // `sizeof(c)` is not taken
+    not exists(SizeofOperator so |
+      so.(SizeofTypeOperator).getTypeOperand().getUnspecifiedType() = c or
+      so.(SizeofExprOperator).getExprOperand().getUnspecifiedType() = c
+    )
+    or
+    // or `sizeof(c)` is taken
     exists(SizeofOperator so |
-      // `sizeof(c)` is taken
       so.(SizeofTypeOperator).getTypeOperand().getUnspecifiedType() = c or
       so.(SizeofExprOperator).getExprOperand().getUnspecifiedType() = c
     |
-      // arithmetic is performed on the result
+      // and arithmetic is performed on the result
       so.getParent*() instanceof AddExpr
     )
-    or
-    exists(AddressOfExpr aoe |
-      // `&(c.v)` is taken
-      aoe.getAddressable() = v
-    )
-    or
-    exists(BuiltInOperationBuiltInOffsetOf oo |
-      // `offsetof(c, v)` using a builtin
-      oo.getAChild().(VariableAccess).getTarget() = v
-    )
   )
 }
 
@@ -61,6 +60,10 @@ int getBufferSize(Expr bufferExpr, Element why) {
     result = bufferVar.getUnspecifiedType().(ArrayType).getSize() and
     why = bufferVar and
     not memberMayBeVarSize(_, bufferVar) and
+    not exists(Union bufferType |
+      bufferType.getAMemberVariable() = why and
+      bufferVar.getUnspecifiedType().(ArrayType).getSize() <= 1
+    ) and
     not result = 0 // zero sized arrays are likely to have special usage, for example
     or
     // behaving a bit like a 'union' overlapping other fields.
@@ -82,6 +85,13 @@ int getBufferSize(Expr bufferExpr, Element why) {
       parentPtr.getTarget().getUnspecifiedType().(PointerType).getBaseType() = parentClass and
       result = getBufferSize(parentPtr, _) + bufferVar.getType().getSize() - parentClass.getSize()
     )
+    or
+    exists(Union bufferType |
+      bufferType.getAMemberVariable() = why and
+      why = bufferVar and
+      bufferVar.getUnspecifiedType().(ArrayType).getSize() <= 1 and
+      result = bufferType.getSize()
+    )
   )
   or
   // buffer is a fixed size dynamic allocation

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll

@@ -3643,9 +3643,10 @@ private module Subpaths {
     PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
-    pathThroughCallable(arg, out, _, apout) and
+    pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
     pathIntoCallable(arg, par, _, innercc, sc, _) and
-    paramFlowsThrough(kind, innercc, sc, apout, _, unbindConf(arg.getConfiguration()))
+    paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _,
+      unbindConf(arg.getConfiguration()))
   }
 
   /**
@@ -3690,8 +3691,8 @@ private module Subpaths {
    */
   predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
-      arg.getASuccessor() = par and
-      arg.getASuccessor() = out and
+      pragma[only_bind_into](arg).getASuccessor() = par and
+      pragma[only_bind_into](arg).getASuccessor() = out and
       subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll

@@ -3643,9 +3643,10 @@ private module Subpaths {
     PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
-    pathThroughCallable(arg, out, _, apout) and
+    pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
     pathIntoCallable(arg, par, _, innercc, sc, _) and
-    paramFlowsThrough(kind, innercc, sc, apout, _, unbindConf(arg.getConfiguration()))
+    paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _,
+      unbindConf(arg.getConfiguration()))
   }
 
   /**
@@ -3690,8 +3691,8 @@ private module Subpaths {
    */
   predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
-      arg.getASuccessor() = par and
-      arg.getASuccessor() = out and
+      pragma[only_bind_into](arg).getASuccessor() = par and
+      pragma[only_bind_into](arg).getASuccessor() = out and
       subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll

@@ -3643,9 +3643,10 @@ private module Subpaths {
     PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
-    pathThroughCallable(arg, out, _, apout) and
+    pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
     pathIntoCallable(arg, par, _, innercc, sc, _) and
-    paramFlowsThrough(kind, innercc, sc, apout, _, unbindConf(arg.getConfiguration()))
+    paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _,
+      unbindConf(arg.getConfiguration()))
   }
 
   /**
@@ -3690,8 +3691,8 @@ private module Subpaths {
    */
   predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
-      arg.getASuccessor() = par and
-      arg.getASuccessor() = out and
+      pragma[only_bind_into](arg).getASuccessor() = par and
+      pragma[only_bind_into](arg).getASuccessor() = out and
       subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll

@@ -3643,9 +3643,10 @@ private module Subpaths {
     PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
-    pathThroughCallable(arg, out, _, apout) and
+    pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
     pathIntoCallable(arg, par, _, innercc, sc, _) and
-    paramFlowsThrough(kind, innercc, sc, apout, _, unbindConf(arg.getConfiguration()))
+    paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _,
+      unbindConf(arg.getConfiguration()))
   }
 
   /**
@@ -3690,8 +3691,8 @@ private module Subpaths {
    */
   predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
-      arg.getASuccessor() = par and
-      arg.getASuccessor() = out and
+      pragma[only_bind_into](arg).getASuccessor() = par and
+      pragma[only_bind_into](arg).getASuccessor() = out and
       subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll

@@ -3643,9 +3643,10 @@ private module Subpaths {
     PathNode arg, ParamNodeEx par, SummaryCtxSome sc, CallContext innercc, ReturnKindExt kind,
     NodeEx out, AccessPath apout
   ) {
-    pathThroughCallable(arg, out, _, apout) and
+    pathThroughCallable(arg, out, _, pragma[only_bind_into](apout)) and
     pathIntoCallable(arg, par, _, innercc, sc, _) and
-    paramFlowsThrough(kind, innercc, sc, apout, _, unbindConf(arg.getConfiguration()))
+    paramFlowsThrough(kind, innercc, sc, pragma[only_bind_into](apout), _,
+      unbindConf(arg.getConfiguration()))
   }
 
   /**
@@ -3690,8 +3691,8 @@ private module Subpaths {
    */
   predicate subpaths(PathNode arg, PathNodeImpl par, PathNodeMid ret, PathNodeMid out) {
     exists(ParamNodeEx p, NodeEx o, AccessPath apout |
-      arg.getASuccessor() = par and
-      arg.getASuccessor() = out and
+      pragma[only_bind_into](arg).getASuccessor() = par and
+      pragma[only_bind_into](arg).getASuccessor() = out and
       subpaths03(arg, p, ret, o, apout) and
       par.getNodeEx() = p and
       out.getNodeEx() = o and