Merge remote-tracking branch 'upstream/master' into dbartol/static-locals

cpp/ql/src/Critical/NewDelete.qll

@@ -12,6 +12,7 @@ import semmle.code.cpp.dataflow.DataFlow
  */
 predicate allocExpr(Expr alloc, string kind) {
   isAllocationExpr(alloc) and
+  not alloc.isFromUninstantiatedTemplate(_) and
   (
     alloc instanceof FunctionCall and
     kind = "malloc"

cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll

@@ -19,15 +19,19 @@ class ConstantZero extends Expr {
  * Holds if `candidate` is an expression such that if it's unsigned then we
  * want an alert at `ge`.
  */
-private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) {
-  // Base case: `candidate >= 0`
-  ge.getRightOperand() instanceof ConstantZero and
-  candidate = ge.getLeftOperand().getFullyConverted() and
-  // left operand was a signed or unsigned IntegralType before conversions
+private predicate lookForUnsignedAt(RelationalOperation ge, Expr candidate) {
+  // Base case: `candidate >= 0` (or `0 <= candidate`)
+  (
+    ge instanceof GEExpr or
+    ge instanceof LEExpr
+  ) and
+  ge.getLesserOperand() instanceof ConstantZero and
+  candidate = ge.getGreaterOperand().getFullyConverted() and
+  // left/greater operand was a signed or unsigned IntegralType before conversions
   // (not a pointer, checking a pointer >= 0 is an entirely different mistake)
   // (not an enum, as the fully converted type of an enum is compiler dependent
   //  so checking an enum >= 0 is always reasonable)
-  ge.getLeftOperand().getUnderlyingType() instanceof IntegralType
+  ge.getGreaterOperand().getUnderlyingType() instanceof IntegralType
   or
   // Recursive case: `...(largerType)candidate >= 0`
   exists(Conversion conversion |
@@ -37,7 +41,7 @@ private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) {
   )
 }
 
-class UnsignedGEZero extends GEExpr {
+class UnsignedGEZero extends ComparisonOperation {
   UnsignedGEZero() {
     exists(Expr ue |
       lookForUnsignedAt(this, ue) and

cpp/ql/src/experimental/Security/CWE/CWE-273/PrivilegeDroppingOutoforder.c

@@ -0,0 +1,138 @@
+#include <stdlib.h>
+#include <sys/param.h>
+#include <unistd.h>
+#include <pwd.h>
+
+void callSetuidAndCheck(int uid) {
+    if (setuid(uid) != 0) {
+        exit(1);
+    }
+}
+
+void callSetgidAndCheck(int gid) {
+    if (setgid(gid) != 0) {
+        exit(1);
+    }
+}
+
+/// Correct ways to drop priv.
+
+void correctDropPrivInline() {
+    if (setgroups(0, NULL)) {
+        exit(1);
+    }
+
+    if (setgid(-2) != 0) {
+        exit(1);
+    }
+
+    if (setuid(-2) != 0) {
+        exit(1);
+    }
+}
+
+void correctDropPrivInScope() {
+    {
+        if (setgroups(0, NULL)) {
+            exit(1);
+        }
+    }
+
+    {
+        if (setgid(-2) != 0) {
+            exit(1);
+        }
+    }
+
+    {
+        if (setuid(-2) != 0) {
+            exit(1);
+        }
+    }
+}
+
+void correctOrderForInitgroups() {
+    struct passwd *pw = getpwuid(0);
+    if (pw) {
+        if (initgroups(pw->pw_name, -2)) {
+            exit(1);
+        }
+    } else {
+        // Unhandled.
+    }
+    int rc = setuid(-2);
+    if (rc) {
+        exit(1);
+    }
+}
+
+void correctDropPrivInScopeParent() {
+    {
+        callSetgidAndCheck(-2);
+    }
+    correctOrderForInitgroups();
+}
+
+void incorrectNoReturnCodeCheck() {
+    int user = -2;
+    if (user) {
+        if (user) {
+            int rc = setgid(user);
+            (void)rc;
+            initgroups("nobody", user);
+        }
+        if (user) {
+            setuid(user);
+        }
+    }
+}
+
+void correctDropPrivInFunctionCall() {
+    if (setgroups(0, NULL)) {
+        exit(1);
+    }
+
+    callSetgidAndCheck(-2);
+    callSetuidAndCheck(-2);
+}
+
+/// Incorrect, out of order gid and uid.
+/// Calling uid before gid will fail.
+
+void incorrectDropPrivOutOfOrderInline() {
+    if (setuid(-2) != 0) {
+        exit(1);
+    }
+
+    if (setgid(-2) != 0) {
+        exit(1);
+    }
+}
+
+void incorrectDropPrivOutOfOrderInScope() {
+    {
+        if (setuid(-2) != 0) {
+            exit(1);
+        }
+    }
+
+    setgid(-2);
+}
+
+void incorrectDropPrivOutOfOrderWithFunction() {
+    callSetuidAndCheck(-2);
+
+    if (setgid(-2) != 0) {
+        exit(1);
+    }
+}
+
+void incorrectDropPrivOutOfOrderWithFunction2() {
+    callSetuidAndCheck(-2);
+    callSetgidAndCheck(-2);
+}
+
+void incorrectDropPrivNoCheck() {
+    setgid(-2);
+    setuid(-2);
+}

cpp/ql/src/experimental/Security/CWE/CWE-273/PrivilegeDroppingOutoforder.qhelp

@@ -0,0 +1,35 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The code attempts to drop privilege in an incorrect order by
+erroneous dropping user privilege before groups. This has security
+impact if the return codes are not checked.</p>
+
+<p>False positives include code performing negative checks, making
+sure that setgid or setgroups does not work, meaning permissions are
+dropped. Additionally, other forms of sandboxing may be present removing
+any residual risk, for example a dedicated user namespace.</p>
+
+</overview>
+<recommendation>
+
+<p>Set the new group ID, then set the target user's intended groups by
+dropping previous supplemental source groups and initializing target
+groups, and finally set the target user.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates out of order calls.</p>
+<sample src="PrivilegeDroppingOutoforder.c" />
+
+</example>
+<references>
+
+<li>CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/POS37-C.+Ensure+that+privilege+relinquishment+is+successful">POS37-C. Ensure that privilege relinquishment is successful</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-273/PrivilegeDroppingOutoforder.ql

@@ -0,0 +1,101 @@
+/**
+ * @name LinuxPrivilegeDroppingOutoforder
+ * @description A syscall commonly associated with privilege dropping is being called out of order.
+ *                Normally a process drops group ID and sets supplimental groups for the target user
+ *                before setting the target user ID. This can have security impact if the return code
+ *                from these methods is not checked.
+ * @kind problem
+ * @problem.severity recommendation
+ * @id cpp/drop-linux-privileges-outoforder
+ * @tags security
+ *       external/cwe/cwe-273
+ * @precision medium
+ */
+
+import cpp
+
+predicate argumentMayBeRoot(Expr e) {
+  e.getValue() = "0" or
+  e.(VariableAccess).getTarget().getName().toLowerCase().matches("%root%")
+}
+
+class SetuidLikeFunctionCall extends FunctionCall {
+  SetuidLikeFunctionCall() {
+    (getTarget().hasGlobalName("setuid") or getTarget().hasGlobalName("setresuid")) and
+    // setuid/setresuid with the root user are false positives.
+    not argumentMayBeRoot(getArgument(0))
+  }
+}
+
+class SetuidLikeWrapperCall extends FunctionCall {
+  SetuidLikeFunctionCall baseCall;
+
+  SetuidLikeWrapperCall() {
+    this = baseCall
+    or
+    exists(SetuidLikeWrapperCall fc |
+      this.getTarget() = fc.getEnclosingFunction() and
+      baseCall = fc.getBaseCall()
+    )
+  }
+
+  SetuidLikeFunctionCall getBaseCall() { result = baseCall }
+}
+
+class CallBeforeSetuidFunctionCall extends FunctionCall {
+  CallBeforeSetuidFunctionCall() {
+    (
+      getTarget().hasGlobalName("setgid") or
+      getTarget().hasGlobalName("setresgid") or
+      // Compatibility may require skipping initgroups and setgroups return checks.
+      // A stricter best practice is to check the result and errnor for EPERM.
+      getTarget().hasGlobalName("initgroups") or
+      getTarget().hasGlobalName("setgroups")
+    ) and
+    // setgid/setresgid/etc with the root group are false positives.
+    not argumentMayBeRoot(getArgument(0))
+  }
+}
+
+class CallBeforeSetuidWrapperCall extends FunctionCall {
+  CallBeforeSetuidFunctionCall baseCall;
+
+  CallBeforeSetuidWrapperCall() {
+    this = baseCall
+    or
+    exists(CallBeforeSetuidWrapperCall fc |
+      this.getTarget() = fc.getEnclosingFunction() and
+      baseCall = fc.getBaseCall()
+    )
+  }
+
+  CallBeforeSetuidFunctionCall getBaseCall() { result = baseCall }
+}
+
+predicate setuidBeforeSetgid(
+  SetuidLikeWrapperCall setuidWrapper, CallBeforeSetuidWrapperCall setgidWrapper
+) {
+  setgidWrapper.getAPredecessor+() = setuidWrapper
+}
+
+predicate isAccessed(FunctionCall fc) {
+  exists(Variable v | v.getAnAssignedValue() = fc)
+  or
+  exists(Operation c | fc = c.getAChild() | c.isCondition())
+  or
+  // ignore pattern where result is intentionally ignored by a cast to void.
+  fc.hasExplicitConversion()
+}
+
+from Function func, CallBeforeSetuidFunctionCall fc, SetuidLikeFunctionCall setuid
+where
+  setuidBeforeSetgid(setuid, fc) and
+  // Require the call return code to be used in a condition or assigned.
+  // This introduces false negatives where the return is checked but then
+  // errno == EPERM allows execution to continue.
+  not isAccessed(fc) and
+  func = fc.getEnclosingFunction()
+select fc,
+  "This function is called within " + func + ", and potentially after " +
+    "$@, and may not succeed. Be sure to check the return code and errno, otherwise permissions " +
+    "may not be dropped.", setuid, setuid.getTarget().getName()

cpp/ql/src/semmle/code/cpp/exprs/ArithmeticOperation.qll

@@ -16,7 +16,7 @@ class UnaryMinusExpr extends UnaryArithmeticOperation, @arithnegexpr {
 
   override string getCanonicalQLClass() { result = "UnaryMinusExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**
@@ -30,7 +30,7 @@ class UnaryPlusExpr extends UnaryArithmeticOperation, @unaryplusexpr {
 
   override string getCanonicalQLClass() { result = "UnaryPlusExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**
@@ -109,7 +109,7 @@ class PrefixIncrExpr extends IncrementOperation, PrefixCrementOperation, @preinc
 
   override string getCanonicalQLClass() { result = "PrefixIncrExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**
@@ -125,7 +125,7 @@ class PrefixDecrExpr extends DecrementOperation, PrefixCrementOperation, @predec
 
   override string getCanonicalQLClass() { result = "PrefixDecrExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**
@@ -141,7 +141,7 @@ class PostfixIncrExpr extends IncrementOperation, PostfixCrementOperation, @post
 
   override string getCanonicalQLClass() { result = "PostfixIncrExpr" }
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 
   override string toString() { result = "... " + getOperator() }
 }
@@ -159,7 +159,7 @@ class PostfixDecrExpr extends DecrementOperation, PostfixCrementOperation, @post
 
   override string getCanonicalQLClass() { result = "PostfixDecrExpr" }
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 
   override string toString() { result = "... " + getOperator() }
 }
@@ -210,7 +210,7 @@ class AddExpr extends BinaryArithmeticOperation, @addexpr {
 
   override string getCanonicalQLClass() { result = "AddExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -224,7 +224,7 @@ class SubExpr extends BinaryArithmeticOperation, @subexpr {
 
   override string getCanonicalQLClass() { result = "SubExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -238,7 +238,7 @@ class MulExpr extends BinaryArithmeticOperation, @mulexpr {
 
   override string getCanonicalQLClass() { result = "MulExpr" }
 
-  override int getPrecedence() { result = 13 }
+  override int getPrecedence() { result = 14 }
 }
 
 /**
@@ -252,7 +252,7 @@ class DivExpr extends BinaryArithmeticOperation, @divexpr {
 
   override string getCanonicalQLClass() { result = "DivExpr" }
 
-  override int getPrecedence() { result = 13 }
+  override int getPrecedence() { result = 14 }
 }
 
 /**
@@ -266,7 +266,7 @@ class RemExpr extends BinaryArithmeticOperation, @remexpr {
 
   override string getCanonicalQLClass() { result = "RemExpr" }
 
-  override int getPrecedence() { result = 13 }
+  override int getPrecedence() { result = 14 }
 }
 
 /**
@@ -283,7 +283,7 @@ class ImaginaryMulExpr extends BinaryArithmeticOperation, @jmulexpr {
 
   override string getCanonicalQLClass() { result = "ImaginaryMulExpr" }
 
-  override int getPrecedence() { result = 13 }
+  override int getPrecedence() { result = 14 }
 }
 
 /**
@@ -300,7 +300,7 @@ class ImaginaryDivExpr extends BinaryArithmeticOperation, @jdivexpr {
 
   override string getCanonicalQLClass() { result = "ImaginaryDivExpr" }
 
-  override int getPrecedence() { result = 13 }
+  override int getPrecedence() { result = 14 }
 }
 
 /**
@@ -318,7 +318,7 @@ class RealImaginaryAddExpr extends BinaryArithmeticOperation, @fjaddexpr {
 
   override string getCanonicalQLClass() { result = "RealImaginaryAddExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -336,7 +336,7 @@ class ImaginaryRealAddExpr extends BinaryArithmeticOperation, @jfaddexpr {
 
   override string getCanonicalQLClass() { result = "ImaginaryRealAddExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -354,7 +354,7 @@ class RealImaginarySubExpr extends BinaryArithmeticOperation, @fjsubexpr {
 
   override string getCanonicalQLClass() { result = "RealImaginarySubExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -372,7 +372,7 @@ class ImaginaryRealSubExpr extends BinaryArithmeticOperation, @jfsubexpr {
 
   override string getCanonicalQLClass() { result = "ImaginaryRealSubExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -416,7 +416,7 @@ class PointerAddExpr extends PointerArithmeticOperation, @paddexpr {
 
   override string getCanonicalQLClass() { result = "PointerAddExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -431,7 +431,7 @@ class PointerSubExpr extends PointerArithmeticOperation, @psubexpr {
 
   override string getCanonicalQLClass() { result = "PointerSubExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }
 
 /**
@@ -446,5 +446,5 @@ class PointerDiffExpr extends PointerArithmeticOperation, @pdiffexpr {
 
   override string getCanonicalQLClass() { result = "PointerDiffExpr" }
 
-  override int getPrecedence() { result = 12 }
+  override int getPrecedence() { result = 13 }
 }

cpp/ql/src/semmle/code/cpp/exprs/BitwiseOperation.qll

@@ -14,7 +14,7 @@ class UnaryBitwiseOperation extends UnaryOperation, @un_bitwise_op_expr { }
 class ComplementExpr extends UnaryBitwiseOperation, @complementexpr {
   override string getOperator() { result = "~" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 
   override string getCanonicalQLClass() { result = "ComplementExpr" }
 }
@@ -33,7 +33,7 @@ class BinaryBitwiseOperation extends BinaryOperation, @bin_bitwise_op_expr { }
 class LShiftExpr extends BinaryBitwiseOperation, @lshiftexpr {
   override string getOperator() { result = "<<" }
 
-  override int getPrecedence() { result = 11 }
+  override int getPrecedence() { result = 12 }
 
   override string getCanonicalQLClass() { result = "LShiftExpr" }
 }
@@ -47,7 +47,7 @@ class LShiftExpr extends BinaryBitwiseOperation, @lshiftexpr {
 class RShiftExpr extends BinaryBitwiseOperation, @rshiftexpr {
   override string getOperator() { result = ">>" }
 
-  override int getPrecedence() { result = 11 }
+  override int getPrecedence() { result = 12 }
 
   override string getCanonicalQLClass() { result = "RShiftExpr" }
 }

cpp/ql/src/semmle/code/cpp/exprs/Call.qll

@@ -74,7 +74,7 @@ abstract class Call extends Expr, NameQualifiableElement {
    */
   abstract Function getTarget();
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 
   override string toString() { none() }
 

cpp/ql/src/semmle/code/cpp/exprs/Cast.qll

@@ -84,7 +84,7 @@ class CStyleCast extends Cast, @c_style_cast {
 
   override string getCanonicalQLClass() { result = "CStyleCast" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**
@@ -103,7 +103,7 @@ class StaticCast extends Cast, @static_cast {
 
   override string getCanonicalQLClass() { result = "StaticCast" }
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 }
 
 /**
@@ -121,7 +121,7 @@ class ConstCast extends Cast, @const_cast {
 
   override string getCanonicalQLClass() { result = "ConstCast" }
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 }
 
 /**
@@ -139,7 +139,7 @@ class ReinterpretCast extends Cast, @reinterpret_cast {
 
   override string getCanonicalQLClass() { result = "ReinterpretCast" }
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 }
 
 private predicate isArithmeticOrEnum(Type type) {
@@ -608,7 +608,7 @@ class PrvalueAdjustmentConversion extends Cast {
 class DynamicCast extends Cast, @dynamic_cast {
   override string toString() { result = "dynamic_cast<" + this.getType().getName() + ">..." }
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 
   override string getCanonicalQLClass() { result = "DynamicCast" }
 
@@ -631,7 +631,7 @@ class UuidofOperator extends Expr, @uuidof {
     else result = "__uuidof(0)"
   }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 
   /** Gets the contained type. */
   Type getTypeOperand() { uuidof_bind(underlyingElement(this), unresolveElement(result)) }
@@ -669,7 +669,7 @@ class TypeidOperator extends Expr, @type_id {
 
   override string toString() { result = "typeid ..." }
 
-  override int getPrecedence() { result = 16 }
+  override int getPrecedence() { result = 17 }
 
   override predicate mayBeImpure() { this.getExpr().mayBeImpure() }
 
@@ -700,7 +700,7 @@ class SizeofPackOperator extends Expr, @sizeof_pack {
  * A C/C++ sizeof expression.
  */
 abstract class SizeofOperator extends Expr, @runtime_sizeof {
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**
@@ -763,7 +763,7 @@ class SizeofTypeOperator extends SizeofOperator {
  * A C++11 `alignof` expression.
  */
 abstract class AlignofOperator extends Expr, @runtime_alignof {
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

@@ -642,7 +642,7 @@ class AddressOfExpr extends UnaryOperation, @address_of {
 
   override string getOperator() { result = "&" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 
   override predicate mayBeImpure() { this.getOperand().mayBeImpure() }
 
@@ -664,7 +664,7 @@ class ReferenceToExpr extends Conversion, @reference_to {
 
   override string getCanonicalQLClass() { result = "ReferenceToExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**
@@ -687,7 +687,7 @@ class PointerDereferenceExpr extends UnaryOperation, @indirect {
 
   override string getOperator() { result = "*" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 
   override predicate mayBeImpure() {
     this.getChild(0).mayBeImpure() or
@@ -721,7 +721,7 @@ class ReferenceDereferenceExpr extends Conversion, @ref_indirect {
  * A C++ `new` or `new[]` expression.
  */
 class NewOrNewArrayExpr extends Expr, @any_new_expr {
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 
   /**
    * Gets the `operator new` or `operator new[]` that allocates storage.
@@ -898,7 +898,7 @@ class DeleteExpr extends Expr, @delete_expr {
 
   override string getCanonicalQLClass() { result = "DeleteExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 
   /**
    * Gets the compile-time type of the object being deleted.
@@ -972,7 +972,7 @@ class DeleteArrayExpr extends Expr, @delete_array_expr {
 
   override string getCanonicalQLClass() { result = "DeleteArrayExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 
   /**
    * Gets the element type of the array being deleted.
@@ -1216,3 +1216,18 @@ private predicate constantTemplateLiteral(Expr e) {
   or
   constantTemplateLiteral(e.(Cast).getExpr())
 }
+
+/**
+ * A C++ three-way comparison operation, also known as the _spaceship
+ * operation_.  This is specific to C++20 and later.
+ * ```
+ * auto c = (a <=> b);
+ * ```
+ */
+class SpaceshipExpr extends BinaryOperation, @spaceshipexpr {
+  override string getCanonicalQLClass() { result = "SpaceshipExpr" }
+
+  override int getPrecedence() { result = 11 }
+
+  override string getOperator() { result = "<=>" }
+}

cpp/ql/src/semmle/code/cpp/exprs/LogicalOperation.qll

@@ -16,7 +16,7 @@ class NotExpr extends UnaryLogicalOperation, @notexpr {
 
   override string getCanonicalQLClass() { result = "NotExpr" }
 
-  override int getPrecedence() { result = 15 }
+  override int getPrecedence() { result = 16 }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

@@ -93,17 +93,27 @@ private float wideningUpperBounds(ArithmeticType t) {
 
 /**
  * Gets the value of the expression `e`, if it is a constant.
- * This predicate also handles the case of constant variables initialized in compilation units,
- * which doesn't necessarily have a getValue() result from the extractor.
+ * This predicate also handles the case of constant variables initialized in different
+ * compilation units, which doesn't necessarily have a getValue() result from the extractor.
  */
 private string getValue(Expr e) {
   if exists(e.getValue())
   then result = e.getValue()
   else
-    exists(VariableAccess access, Variable v |
+    /*
+     * It should be safe to propagate the initialization value to a variable if:
+     * The type of v is const, and
+     * The type of v is not volatile, and
+     * Either:
+     *   v is a local/global variable, or
+     *   v is a static member variable
+     */
+
+    exists(VariableAccess access, StaticStorageDurationVariable v |
+      not v.getUnderlyingType().isVolatile() and
+      v.getUnderlyingType().isConst() and
       e = access and
       v = access.getTarget() and
-      v.getUnderlyingType().isConst() and
       result = getValue(v.getAnAssignedValue())
     )
 }

cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll

@@ -99,6 +99,8 @@ class ArrayExecFunctionCall extends FunctionCall {
     getTarget().hasGlobalName("execv") or
     getTarget().hasGlobalName("execvp") or
     getTarget().hasGlobalName("execvpe") or
+    getTarget().hasGlobalName("execve") or
+    getTarget().hasGlobalName("fexecve") or
     // Windows variants
     getTarget().hasGlobalName("_execv") or
     getTarget().hasGlobalName("_execve") or

cpp/ql/src/semmlecode.cpp.dbscheme

@@ -1231,6 +1231,7 @@ funbind(
              | @ltexpr
              | @geexpr
              | @leexpr
+             | @spaceshipexpr
              ;
 
 @bin_bitwise_op_expr = @lshiftexpr
@@ -1636,6 +1637,7 @@ case @expr.kind of
 | 323 = @vec_fill
 | 324 = @builtinconvertvector
 | 325 = @builtincomplex
+| 326 = @spaceshipexpr
 ;
 
 new_allocated_type(

cpp/ql/test/library-tests/multiple_declarations/variable_types/extracted_once.h

@@ -0,0 +1,16 @@
+// This header file is extracted only once even though it's included by both
+// file1.c and file2.c. That's presumably because it's wrongly considered to
+// expand to the same trap in both contexts. In practice, this header gets
+// extracted together with the extraction of file1.c.
+
+// BUG: types of members depend on extraction order.
+// Only one copy of this struct is extracted, and the types of its members refer
+// to the typedefs in file1.c. Had file2.c been extracted first instead, the
+// types of its members would be different.
+struct UnifiableOnce {
+    intAlias intMember;
+    qualifiedIntAlias qualifiedIntMember;
+};
+
+// BUG: types of parameters depend on extraction order.
+void functionOnce(intAlias param);

cpp/ql/test/library-tests/multiple_declarations/variable_types/extracted_twice.h

@@ -0,0 +1,30 @@
+// This header file is extracted twice because its inclusions in file1.c and
+// file2.c lead to different context hashes, seemingly because this file (unlike
+// extracted_once.h) refers to `structAlias`. That means the resulting trap has
+// two copies of all declarations in this file, and those copies have to be
+// unified in the trap import step or in QL.
+
+// GOOD. The types of the members of this struct are unifiable, which in this
+// context means that they share the same unspecified types. This means that the
+// two extractions of the struct get the same content hash and therefore become
+// one entry in the database. Both struct members have multiple types in the
+// `membervariables` table, but those are unified in the
+// `MemberVariable.getType()` predicate.
+struct UnifiableTwice {
+    intAlias intMember;
+    qualifiedIntAlias qualifiedIntMember;
+};
+
+// BUG: Non-member variables of this type have two types in the database.
+// The type of `structMember` is ambiguous, and the two possible types are not
+// unifiable, meaning in this context that they don't share an unspecified type.
+// The types are nevertheless _compatible_, so it's valid C (not C++) to use
+// these two definitions interchangably in the same program.
+struct NotUnifiableTwice {
+    structAlias structMember;
+};
+
+// BUG: The parameter of this function has two types.
+// Because the `MemberVariable.getType()` workaround does not apply to a
+// `Parameter`, this `Parameter` gets two types.
+void functionTwice(intAlias param);

cpp/ql/test/library-tests/multiple_declarations/variable_types/file1.c

@@ -0,0 +1,15 @@
+// These typedefs are all _compatible_ (see
+// https://en.cppreference.com/w/c/language/type#Compatible_types) with their
+// siblings in file2.c. It varies whether they have a canonical form that's
+// common to them both.
+typedef int localInt;
+typedef localInt intAlias; // has common `getUnderlyingType()` and `getUnspecifiedType()`
+typedef int qualifiedIntAlias; // only has common `getUnspecifiedType()`
+typedef struct emptyStruct1 { } structAlias; // has no common type
+
+#include "extracted_once.h"
+struct UnifiableOnce uOnce;
+
+#include "extracted_twice.h"
+struct UnifiableTwice uTwice;
+struct NotUnifiableTwice nTwice; // BUG: this variable has two types

cpp/ql/test/library-tests/multiple_declarations/variable_types/file2.c

@@ -0,0 +1,10 @@
+typedef int intAlias;
+typedef const int qualifiedIntAlias;
+typedef struct emptyStruct2 { } structAlias;
+
+#include "extracted_once.h"
+struct UnifiableOnce uOnce;
+
+#include "extracted_twice.h"
+struct UnifiableTwice uTwice;
+struct NotUnifiableTwice nTwice; // BUG: this variable has two types

cpp/ql/test/library-tests/multiple_declarations/variable_types/varType.expected

@@ -0,0 +1,17 @@
+| extracted_once.h:11:14:11:22 | intMember | file1.c:6:18:6:25 | intAlias | 1 |
+| extracted_once.h:12:23:12:40 | qualifiedIntMember | file1.c:7:13:7:29 | qualifiedIntAlias | 1 |
+| extracted_once.h:16:28:16:32 | param | file1.c:6:18:6:25 | intAlias | 1 |
+| extracted_twice.h:14:14:14:22 | intMember | file://:0:0:0:0 | int | 1 |
+| extracted_twice.h:15:23:15:40 | qualifiedIntMember | file://:0:0:0:0 | int | 1 |
+| extracted_twice.h:24:17:24:28 | structMember | file1.c:8:33:8:43 | structAlias | 1 |
+| extracted_twice.h:24:17:24:28 | structMember | file2.c:3:33:3:43 | structAlias | 1 |
+| extracted_twice.h:30:29:30:33 | param | file1.c:6:18:6:25 | intAlias | 2 |
+| extracted_twice.h:30:29:30:33 | param | file2.c:1:13:1:20 | intAlias | 2 |
+| file1.c:11:22:11:26 | uOnce | extracted_once.h:10:8:10:20 | UnifiableOnce | 1 |
+| file1.c:14:23:14:28 | uTwice | extracted_twice.h:13:8:13:21 | UnifiableTwice | 1 |
+| file1.c:15:26:15:31 | nTwice | extracted_twice.h:23:8:23:24 | NotUnifiableTwice | 2 |
+| file1.c:15:26:15:31 | nTwice | extracted_twice.h:23:8:23:24 | NotUnifiableTwice | 2 |
+| file2.c:6:22:6:26 | uOnce | extracted_once.h:10:8:10:20 | UnifiableOnce | 1 |
+| file2.c:9:23:9:28 | uTwice | extracted_twice.h:13:8:13:21 | UnifiableTwice | 1 |
+| file2.c:10:26:10:31 | nTwice | extracted_twice.h:23:8:23:24 | NotUnifiableTwice | 2 |
+| file2.c:10:26:10:31 | nTwice | extracted_twice.h:23:8:23:24 | NotUnifiableTwice | 2 |

cpp/ql/test/library-tests/multiple_declarations/variable_types/varType.ql

@@ -0,0 +1,5 @@
+import cpp
+
+from Variable var
+where exists(var.getFile().getRelativePath())
+select var, var.getType(), strictcount(var.getType())

cpp/ql/test/query-tests/Critical/NewFree/NewFreeMismatch.expected

@@ -1,3 +1,5 @@
+| test2.cpp:19:3:19:6 | call to free | There is a new/free mismatch between this free and the corresponding $@. | test2.cpp:18:12:18:18 | new | new |
+| test2.cpp:26:3:26:6 | call to free | There is a new/free mismatch between this free and the corresponding $@. | test2.cpp:25:7:25:13 | new | new |
 | test.cpp:36:2:36:17 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:27:18:27:23 | call to malloc | malloc |
 | test.cpp:41:2:41:5 | call to free | There is a new/free mismatch between this free and the corresponding $@. | test.cpp:26:7:26:17 | new | new |
 | test.cpp:68:3:68:11 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:64:28:64:33 | call to malloc | malloc |

cpp/ql/test/query-tests/Critical/NewFree/test2.cpp

@@ -0,0 +1,36 @@
+// semmle-extractor-options: -std=gnu++14
+
+typedef unsigned long size_t;
+
+void *malloc(size_t size);
+void free(void *ptr);
+
+void* operator new(size_t _Size, void *_Where);
+
+// ---
+
+template<typename T>
+class MyTest2Class
+{
+public:
+	MyTest2Class()
+	{
+		int *a = new int;
+		free(a); // BAD
+
+		int *ptr_b = (int *)malloc(sizeof(int));
+		int *b = new(ptr_b) int;
+		free(b); // GOOD
+
+		c = new int;
+		free(c); // BAD
+
+		int *ptr_d = (int *)malloc(sizeof(int));
+		d = new(ptr_d) int;
+		free(d); // GOOD
+	}
+
+	int *c, *d;
+};
+
+MyTest2Class<int> mt2c_i;

cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.cpp

@@ -0,0 +1,29 @@
+void func_with_default_arg(const int n = 0) {
+    if(n <= 10) {}
+}
+
+struct A {
+    const int int_member = 0;
+    A(int n) : int_member(n) {
+        if(int_member <= 10) {
+                    
+        }
+    }
+};
+
+struct B {
+    B(const int n = 0) {
+        if(n <= 10) {}
+    }
+};
+
+const volatile int volatile_const_global = 0;
+
+void test1() {
+    func_with_default_arg(100);
+
+    A a(100);
+    if(a.int_member <= 10) {}
+
+    if(volatile_const_global <= 10) {}
+}

cpp/ql/test/query-tests/Likely Bugs/Arithmetic/UnsignedGEZero/UnsignedGEZero.c

@@ -112,4 +112,29 @@ void myFunction() {
 	assert(CHECK_RANGE(ui, 0, 10)); // reasonable use
 	assert(UI >= ZERO); // violation (not detected)
 	assert(ui GE 0); // violation
+
+	if ((unsigned char)si >= 0) { // violation
+	}
+	if ((unsigned char)(signed int)si >= 0) { // violation
+	}
+	if ((signed int)(unsigned char)si >= 0) { // violation
+	}
+	if ((unsigned char)(signed char)si >= 0) { // violation
+	}
+	if ((signed char)(unsigned char)si >= 0) {
+	}
+
+	if ((signed int)(unsigned char)(signed int)si >= 0) { // violation
+	}
+	if ((signed char)(unsigned char)(signed int)si >= 0) {
+	}
+	if ((signed int)(unsigned char)(signed char)si >= 0) { // violation
+	}
+
+	if (ui <= 0) {
+	}
+	if (0 <= ui) { // violation
+	}
+	if (0 < ui) {
+	}
 }

cpp/ql/test/query-tests/Likely Bugs/Arithmetic/UnsignedGEZero/UnsignedGEZero.cpp

@@ -112,4 +112,29 @@ void myFunction() {
 	assert(CHECK_RANGE(ui, 0, 10)); // reasonable use
 	assert(UI >= ZERO); // violation (not detected)
 	assert(ui GE 0); // violation
+
+	if ((unsigned char)si >= 0) { // violation
+	}
+	if ((unsigned char)(signed int)si >= 0) { // violation
+	}
+	if ((signed int)(unsigned char)si >= 0) { // violation
+	}
+	if ((unsigned char)(signed char)si >= 0) { // violation
+	}
+	if ((signed char)(unsigned char)si >= 0) {
+	}
+
+	if ((signed int)(unsigned char)(signed int)si >= 0) { // violation
+	}
+	if ((signed char)(unsigned char)(signed int)si >= 0) {
+	}
+	if ((signed int)(unsigned char)(signed char)si >= 0) { // violation
+	}
+
+	if (ui <= 0) {
+	}
+	if (0 <= ui) { // violation
+	}
+	if (0 < ui) {
+	}
 }

cpp/ql/test/query-tests/Likely Bugs/Arithmetic/UnsignedGEZero/UnsignedGEZero.expected

@@ -14,6 +14,13 @@
 | UnsignedGEZero.c:101:9:101:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
 | UnsignedGEZero.c:111:9:111:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
 | UnsignedGEZero.c:114:9:114:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.c:116:6:116:27 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.c:118:6:118:39 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.c:120:6:120:39 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.c:122:6:122:40 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.c:127:6:127:51 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.c:131:6:131:52 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.c:136:6:136:12 | ... <= ... | Pointless comparison of unsigned value to zero. |
 | UnsignedGEZero.cpp:40:6:40:12 | ... >= ... | Pointless comparison of unsigned value to zero. |
 | UnsignedGEZero.cpp:48:6:48:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
 | UnsignedGEZero.cpp:54:6:54:12 | ... >= ... | Pointless comparison of unsigned value to zero. |
@@ -29,3 +36,10 @@
 | UnsignedGEZero.cpp:101:9:101:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
 | UnsignedGEZero.cpp:111:9:111:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
 | UnsignedGEZero.cpp:114:9:114:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.cpp:116:6:116:27 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.cpp:118:6:118:39 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.cpp:120:6:120:39 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.cpp:122:6:122:40 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.cpp:127:6:127:51 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.cpp:131:6:131:52 | ... >= ... | Pointless comparison of unsigned value to zero. |
+| UnsignedGEZero.cpp:136:6:136:12 | ... <= ... | Pointless comparison of unsigned value to zero. |

cpp/upgrades/c9ac0461491edef3b1ab79f03d007a47522dda90/upgrade.properties

@@ -0,0 +1,2 @@
+description: Add support for C++'s <=> operator
+compatibility: backwards