Merge branch 'master' into ir-flow-fields

2026-04-29 18:55:14 +02:00 · 2020-04-02 15:23:00 +02:00
parent dda3aaa8aa 4825774ce2
commit ce5d8d516f
24 changed files with 525 additions and 53 deletions
--- a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll
+++ b/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll
@@ -2,6 +2,7 @@ import semmle.code.cpp.Element
 private import semmle.code.cpp.Enclosing
 private import semmle.code.cpp.internal.ResolveClass
 private import semmle.code.cpp.internal.AddressConstantExpression
+private import semmle.code.cpp.models.implementations.Allocation

 /**
 * A C/C++ expression.
@@ -804,8 +805,10 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
   * call the constructor of `T` but will not allocate memory.
   */
  Expr getPlacementPointer() {
-    isStandardPlacementNewAllocator(this.getAllocator()) and
-    result = this.getAllocatorCall().getArgument(1)
+    result =
+      this
+          .getAllocatorCall()
+          .getArgument(this.getAllocator().(OperatorNewAllocationFunction).getPlacementArgument())
  }
 }

@@ -1194,12 +1197,6 @@ private predicate convparents(Expr child, int idx, Element parent) {
  )
 }

-private predicate isStandardPlacementNewAllocator(Function operatorNew) {
-  operatorNew.getName().matches("operator new%") and
-  operatorNew.getNumberOfParameters() = 2 and
-  operatorNew.getParameter(1).getType() instanceof VoidPointerType
-}
-
 // Pulled out for performance. See QL-796.
 private predicate hasNoConversions(Expr e) { not e.hasConversion() }

--- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll
+++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll
@@ -343,6 +343,11 @@ class TranslatedSideEffects extends TranslatedElement, TTranslatedSideEffects {

  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) {
    expr.getTarget() instanceof AllocationFunction and
+    not exists(NewOrNewArrayExpr newExpr |
+      // we synthesize allocator calls for `new` and `new[]`, so don't add instructions to
+      // the existing allocator call when it exists.
+      newExpr.getAllocatorCall() = expr
+    ) and
    opcode instanceof Opcode::InitializeDynamicAllocation and
    tag = OnlyInstructionTag() and
    type = getUnknownType()
@@ -358,6 +363,11 @@ class TranslatedSideEffects extends TranslatedElement, TTranslatedSideEffects {
    tag = OnlyInstructionTag() and
    kind = gotoEdge() and
    expr.getTarget() instanceof AllocationFunction and
+    not exists(NewOrNewArrayExpr newExpr |
+      // we synthesize allocator calls for `new` and `new[]`, so don't add instructions to
+      // the existing allocator call when it exists.
+      newExpr.getAllocatorCall() = expr
+    ) and
    if exists(getChild(0))
    then result = getChild(0).getFirstInstruction()
    else result = getParent().getChildSuccessor(this)
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll
@@ -215,6 +215,40 @@ class SizelessAllocationFunction extends AllocationFunction {
  }
 }

+/**
+ * An `operator new` or `operator new[]` function that may be associated with `new` or
+ * `new[]` expressions.  Note that `new` and `new[]` are not function calls, but these
+ * functions may also be called directly.
+ */
+class OperatorNewAllocationFunction extends AllocationFunction {
+  OperatorNewAllocationFunction() {
+    exists(string name |
+      hasGlobalName(name) and
+      (
+        // operator new(bytes, ...)
+        name = "operator new"
+        or
+        // operator new[](bytes, ...)
+        name = "operator new[]"
+      )
+    )
+  }
+
+  override int getSizeArg() { result = 0 }
+
+  override predicate requiresDealloc() { not exists(getPlacementArgument()) }
+
+  /**
+   * Gets the position of the placement pointer if this is a placement
+   * `operator new` function.
+   */
+  int getPlacementArgument() {
+    getNumberOfParameters() = 2 and
+    getParameter(1).getType() instanceof VoidPointerType and
+    result = 1
+  }
+}
+
 /**
 * An allocation expression that is a function call, such as call to `malloc`.
 */
@@ -227,7 +261,9 @@ class CallAllocationExpr extends AllocationExpr, FunctionCall {
    not (
      exists(target.getReallocPtrArg()) and
      getArgument(target.getSizeArg()).getValue().toInt() = 0
-    )
+    ) and
+    // these are modelled directly (and more accurately), avoid duplication
+    not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this)
  }

  override Expr getSizeExpr() { result = getArgument(target.getSizeArg()) }
--- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
+++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll
@@ -79,6 +79,28 @@ class StandardDeallocationFunction extends DeallocationFunction {
  override int getFreedArg() { result = freedArg }
 }

+/**
+ * An `operator delete` or `operator delete[]` function that may be associated
+ * with `delete` or `delete[]` expressions.  Note that `delete` and `delete[]`
+ * are not function calls, but these functions may also be called directly.
+ */
+class OperatorDeleteDeallocationFunction extends DeallocationFunction {
+  OperatorDeleteDeallocationFunction() {
+    exists(string name |
+      hasGlobalName(name) and
+      (
+        // operator delete(pointer, ...)
+        name = "operator delete"
+        or
+        // operator delete[](pointer, ...)
+        name = "operator delete[]"
+      )
+    )
+  }
+
+  override int getFreedArg() { result = 0 }
+}
+
 /**
 * An deallocation expression that is a function call, such as call to `free`.
 */
--- a/cpp/ql/test/library-tests/allocators/allocators.cpp
+++ b/cpp/ql/test/library-tests/allocators/allocators.cpp
@@ -143,3 +143,9 @@ void multidimensionalNew(int x, int y) {
  auto p2 = new char[20][20];
  auto p3 = new char[x][30][30];
 }
+
+void directOperatorCall() {
+	void *ptr;
+	ptr = operator new(sizeof(int));
+	operator delete(ptr);
+}
--- a/cpp/ql/test/library-tests/allocators/allocators.expected
+++ b/cpp/ql/test/library-tests/allocators/allocators.expected
@@ -1,28 +1,28 @@
 newExprs
-| allocators.cpp:49:3:49:9 | new | int | operator new(unsigned long) -> void * | 4 | 4 |  |
-| allocators.cpp:50:3:50:15 | new | int | operator new(size_t, float) -> void * | 4 | 4 |  |
-| allocators.cpp:51:3:51:11 | new | int | operator new(unsigned long) -> void * | 4 | 4 |  |
-| allocators.cpp:52:3:52:14 | new | String | operator new(unsigned long) -> void * | 8 | 8 |  |
-| allocators.cpp:53:3:53:27 | new | String | operator new(size_t, float) -> void * | 8 | 8 |  |
-| allocators.cpp:54:3:54:17 | new | Overaligned | operator new(unsigned long, align_val_t) -> void * | 256 | 128 | aligned |
-| allocators.cpp:55:3:55:25 | new | Overaligned | operator new(size_t, align_val_t, float) -> void * | 256 | 128 | aligned |
-| allocators.cpp:107:3:107:18 | new | FailedInit | FailedInit::operator new(size_t) -> void * | 1 | 1 |  |
-| allocators.cpp:109:3:109:35 | new | FailedInitOveraligned | FailedInitOveraligned::operator new(size_t, align_val_t, float) -> void * | 128 | 128 | aligned |
-| allocators.cpp:129:3:129:21 | new | int | operator new(size_t, void *) -> void * | 4 | 4 |  |
-| allocators.cpp:135:3:135:26 | new | int | operator new(size_t, const nothrow_t &) -> void * | 4 | 4 |  |
+| allocators.cpp:49:3:49:9 | new | int | operator new(unsigned long) -> void * | 4 | 4 |  |  |
+| allocators.cpp:50:3:50:15 | new | int | operator new(size_t, float) -> void * | 4 | 4 |  |  |
+| allocators.cpp:51:3:51:11 | new | int | operator new(unsigned long) -> void * | 4 | 4 |  |  |
+| allocators.cpp:52:3:52:14 | new | String | operator new(unsigned long) -> void * | 8 | 8 |  |  |
+| allocators.cpp:53:3:53:27 | new | String | operator new(size_t, float) -> void * | 8 | 8 |  |  |
+| allocators.cpp:54:3:54:17 | new | Overaligned | operator new(unsigned long, align_val_t) -> void * | 256 | 128 | aligned |  |
+| allocators.cpp:55:3:55:25 | new | Overaligned | operator new(size_t, align_val_t, float) -> void * | 256 | 128 | aligned |  |
+| allocators.cpp:107:3:107:18 | new | FailedInit | FailedInit::operator new(size_t) -> void * | 1 | 1 |  |  |
+| allocators.cpp:109:3:109:35 | new | FailedInitOveraligned | FailedInitOveraligned::operator new(size_t, align_val_t, float) -> void * | 128 | 128 | aligned |  |
+| allocators.cpp:129:3:129:21 | new | int | operator new(size_t, void *) -> void * | 4 | 4 |  | & ... |
+| allocators.cpp:135:3:135:26 | new | int | operator new(size_t, const nothrow_t &) -> void * | 4 | 4 |  |  |
 newArrayExprs
-| allocators.cpp:68:3:68:12 | new[] | int[] | int | operator new[](unsigned long) -> void * | 4 | 4 |  | n |
-| allocators.cpp:69:3:69:18 | new[] | int[] | int | operator new[](size_t, float) -> void * | 4 | 4 |  | n |
-| allocators.cpp:70:3:70:15 | new[] | String[] | String | operator new[](unsigned long) -> void * | 8 | 8 |  | n |
-| allocators.cpp:71:3:71:20 | new[] | Overaligned[] | Overaligned | operator new[](unsigned long, align_val_t) -> void * | 256 | 128 | aligned | n |
-| allocators.cpp:72:3:72:16 | new[] | String[10] | String | operator new[](unsigned long) -> void * | 8 | 8 |  |  |
-| allocators.cpp:108:3:108:19 | new[] | FailedInit[] | FailedInit | FailedInit::operator new[](size_t) -> void * | 1 | 1 |  | n |
-| allocators.cpp:110:3:110:37 | new[] | FailedInitOveraligned[10] | FailedInitOveraligned | FailedInitOveraligned::operator new[](size_t, align_val_t, float) -> void * | 128 | 128 | aligned |  |
-| allocators.cpp:132:3:132:17 | new[] | int[1] | int | operator new[](size_t, void *) -> void * | 4 | 4 |  |  |
-| allocators.cpp:136:3:136:26 | new[] | int[2] | int | operator new[](size_t, const nothrow_t &) -> void * | 4 | 4 |  |  |
-| allocators.cpp:142:13:142:27 | new[] | char[][10] | char[10] | operator new[](unsigned long) -> void * | 10 | 1 |  | x |
-| allocators.cpp:143:13:143:28 | new[] | char[20][20] | char[20] | operator new[](unsigned long) -> void * | 20 | 1 |  |  |
-| allocators.cpp:144:13:144:31 | new[] | char[][30][30] | char[30][30] | operator new[](unsigned long) -> void * | 900 | 1 |  | x |
+| allocators.cpp:68:3:68:12 | new[] | int[] | int | operator new[](unsigned long) -> void * | 4 | 4 |  | n |  |
+| allocators.cpp:69:3:69:18 | new[] | int[] | int | operator new[](size_t, float) -> void * | 4 | 4 |  | n |  |
+| allocators.cpp:70:3:70:15 | new[] | String[] | String | operator new[](unsigned long) -> void * | 8 | 8 |  | n |  |
+| allocators.cpp:71:3:71:20 | new[] | Overaligned[] | Overaligned | operator new[](unsigned long, align_val_t) -> void * | 256 | 128 | aligned | n |  |
+| allocators.cpp:72:3:72:16 | new[] | String[10] | String | operator new[](unsigned long) -> void * | 8 | 8 |  |  |  |
+| allocators.cpp:108:3:108:19 | new[] | FailedInit[] | FailedInit | FailedInit::operator new[](size_t) -> void * | 1 | 1 |  | n |  |
+| allocators.cpp:110:3:110:37 | new[] | FailedInitOveraligned[10] | FailedInitOveraligned | FailedInitOveraligned::operator new[](size_t, align_val_t, float) -> void * | 128 | 128 | aligned |  |  |
+| allocators.cpp:132:3:132:17 | new[] | int[1] | int | operator new[](size_t, void *) -> void * | 4 | 4 |  |  | buf |
+| allocators.cpp:136:3:136:26 | new[] | int[2] | int | operator new[](size_t, const nothrow_t &) -> void * | 4 | 4 |  |  |  |
+| allocators.cpp:142:13:142:27 | new[] | char[][10] | char[10] | operator new[](unsigned long) -> void * | 10 | 1 |  | x |  |
+| allocators.cpp:143:13:143:28 | new[] | char[20][20] | char[20] | operator new[](unsigned long) -> void * | 20 | 1 |  |  |  |
+| allocators.cpp:144:13:144:31 | new[] | char[][30][30] | char[30][30] | operator new[](unsigned long) -> void * | 900 | 1 |  | x |  |
 newExprDeallocators
 | allocators.cpp:52:3:52:14 | new | String | operator delete(void *, unsigned long) -> void | 8 | 8 | sized  |
 | allocators.cpp:53:3:53:27 | new | String | operator delete(void *, float) -> void | 8 | 8 |   |
@@ -46,3 +46,65 @@ deleteArrayExprs
 | allocators.cpp:81:3:81:45 | delete[] | Overaligned | operator delete[](void *, unsigned long, align_val_t) -> void | 256 | 128 | sized aligned |
 | allocators.cpp:82:3:82:49 | delete[] | PolymorphicBase | operator delete[](void *, unsigned long) -> void | 8 | 8 | sized  |
 | allocators.cpp:83:3:83:23 | delete[] | int | operator delete[](void *, unsigned long) -> void | 4 | 4 | sized  |
+allocationFunctions
+| allocators.cpp:7:7:7:18 | operator new | getSizeArg = 0, requiresDealloc |
+| allocators.cpp:8:7:8:20 | operator new[] | getSizeArg = 0, requiresDealloc |
+| allocators.cpp:9:7:9:18 | operator new | getSizeArg = 0, requiresDealloc |
+| allocators.cpp:10:7:10:20 | operator new[] | getSizeArg = 0, requiresDealloc |
+| allocators.cpp:121:7:121:18 | operator new | getPlacementArgument = 1, getSizeArg = 0 |
+| allocators.cpp:122:7:122:20 | operator new[] | getPlacementArgument = 1, getSizeArg = 0 |
+| allocators.cpp:123:7:123:18 | operator new | getSizeArg = 0, requiresDealloc |
+| allocators.cpp:124:7:124:20 | operator new[] | getSizeArg = 0, requiresDealloc |
+| file://:0:0:0:0 | operator new | getSizeArg = 0, requiresDealloc |
+| file://:0:0:0:0 | operator new | getSizeArg = 0, requiresDealloc |
+| file://:0:0:0:0 | operator new[] | getSizeArg = 0, requiresDealloc |
+| file://:0:0:0:0 | operator new[] | getSizeArg = 0, requiresDealloc |
+allocationExprs
+| allocators.cpp:49:3:49:9 | new | getSizeBytes = 4, requiresDealloc |
+| allocators.cpp:50:3:50:15 | new | getSizeBytes = 4, requiresDealloc |
+| allocators.cpp:51:3:51:11 | new | getSizeBytes = 4, requiresDealloc |
+| allocators.cpp:52:3:52:14 | new | getSizeBytes = 8, requiresDealloc |
+| allocators.cpp:53:3:53:27 | new | getSizeBytes = 8, requiresDealloc |
+| allocators.cpp:54:3:54:17 | new | getSizeBytes = 256, requiresDealloc |
+| allocators.cpp:55:3:55:25 | new | getSizeBytes = 256, requiresDealloc |
+| allocators.cpp:68:3:68:12 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc |
+| allocators.cpp:69:3:69:18 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc |
+| allocators.cpp:70:3:70:15 | new[] | getSizeExpr = n, getSizeMult = 8, requiresDealloc |
+| allocators.cpp:71:3:71:20 | new[] | getSizeExpr = n, getSizeMult = 256, requiresDealloc |
+| allocators.cpp:72:3:72:16 | new[] | getSizeBytes = 80, requiresDealloc |
+| allocators.cpp:107:3:107:18 | new | getSizeBytes = 1, requiresDealloc |
+| allocators.cpp:108:3:108:19 | new[] | getSizeExpr = n, getSizeMult = 1, requiresDealloc |
+| allocators.cpp:109:3:109:35 | new | getSizeBytes = 128, requiresDealloc |
+| allocators.cpp:110:3:110:37 | new[] | getSizeBytes = 1280, requiresDealloc |
+| allocators.cpp:129:3:129:21 | new | getSizeBytes = 4 |
+| allocators.cpp:132:3:132:17 | new[] | getSizeBytes = 4 |
+| allocators.cpp:135:3:135:26 | new | getSizeBytes = 4, requiresDealloc |
+| allocators.cpp:136:3:136:26 | new[] | getSizeBytes = 8, requiresDealloc |
+| allocators.cpp:142:13:142:27 | new[] | getSizeExpr = x, getSizeMult = 10, requiresDealloc |
+| allocators.cpp:143:13:143:28 | new[] | getSizeBytes = 400, requiresDealloc |
+| allocators.cpp:144:13:144:31 | new[] | getSizeExpr = x, getSizeMult = 900, requiresDealloc |
+| allocators.cpp:149:8:149:19 | call to operator new | getSizeBytes = 4, getSizeExpr = sizeof(int), getSizeMult = 1, requiresDealloc |
+deallocationFunctions
+| allocators.cpp:11:6:11:20 | operator delete | getFreedArg = 0 |
+| allocators.cpp:12:6:12:22 | operator delete[] | getFreedArg = 0 |
+| allocators.cpp:13:6:13:20 | operator delete | getFreedArg = 0 |
+| allocators.cpp:14:6:14:22 | operator delete[] | getFreedArg = 0 |
+| file://:0:0:0:0 | operator delete | getFreedArg = 0 |
+| file://:0:0:0:0 | operator delete | getFreedArg = 0 |
+| file://:0:0:0:0 | operator delete | getFreedArg = 0 |
+| file://:0:0:0:0 | operator delete[] | getFreedArg = 0 |
+| file://:0:0:0:0 | operator delete[] | getFreedArg = 0 |
+deallocationExprs
+| allocators.cpp:59:3:59:35 | delete | getFreedExpr = 0 |
+| allocators.cpp:60:3:60:38 | delete | getFreedExpr = 0 |
+| allocators.cpp:61:3:61:44 | delete | getFreedExpr = 0 |
+| allocators.cpp:62:3:62:43 | delete | getFreedExpr = 0 |
+| allocators.cpp:63:3:63:47 | delete | getFreedExpr = 0 |
+| allocators.cpp:64:3:64:44 | delete | getFreedExpr = 0 |
+| allocators.cpp:78:3:78:37 | delete[] | getFreedExpr = 0 |
+| allocators.cpp:79:3:79:40 | delete[] | getFreedExpr = 0 |
+| allocators.cpp:80:3:80:46 | delete[] | getFreedExpr = 0 |
+| allocators.cpp:81:3:81:45 | delete[] | getFreedExpr = 0 |
+| allocators.cpp:82:3:82:49 | delete[] | getFreedExpr = 0 |
+| allocators.cpp:83:3:83:23 | delete[] | getFreedExpr = call to GetPointer |
+| allocators.cpp:150:2:150:16 | call to operator delete | getFreedExpr = ptr |
--- a/cpp/ql/test/library-tests/allocators/allocators.ql
+++ b/cpp/ql/test/library-tests/allocators/allocators.ql
@@ -1,6 +1,9 @@
 import default
+import semmle.code.cpp.models.implementations.Allocation

-query predicate newExprs(NewExpr expr, string type, string sig, int size, int alignment, string form) {
+query predicate newExprs(
+  NewExpr expr, string type, string sig, int size, int alignment, string form, string placement
+) {
  exists(Function allocator, Type allocatedType |
    expr.getAllocator() = allocator and
    sig = allocator.getFullSignature() and
@@ -8,13 +11,16 @@ query predicate newExprs(NewExpr expr, string type, string sig, int size, int al
    type = allocatedType.toString() and
    size = allocatedType.getSize() and
    alignment = allocatedType.getAlignment() and
-    if expr.hasAlignedAllocation() then form = "aligned" else form = ""
+    (if expr.hasAlignedAllocation() then form = "aligned" else form = "") and
+    if exists(expr.getPlacementPointer())
+    then placement = expr.getPlacementPointer().toString()
+    else placement = ""
  )
 }

 query predicate newArrayExprs(
  NewArrayExpr expr, string t1, string t2, string sig, int size, int alignment, string form,
-  string extents
+  string extents, string placement
 ) {
  exists(Function allocator, Type arrayType, Type elementType |
    expr.getAllocator() = allocator and
@@ -26,7 +32,10 @@ query predicate newArrayExprs(
    size = elementType.getSize() and
    alignment = elementType.getAlignment() and
    (if expr.hasAlignedAllocation() then form = "aligned" else form = "") and
-    extents = concat(Expr e | e = expr.getExtent() | e.toString(), ", ")
+    extents = concat(Expr e | e = expr.getExtent() | e.toString(), ", ") and
+    if exists(expr.getPlacementPointer())
+    then placement = expr.getPlacementPointer().toString()
+    else placement = ""
  )
 }

@@ -101,3 +110,54 @@ query predicate deleteArrayExprs(
    )
  )
 }
+
+string describeAllocationFunction(AllocationFunction f) {
+  result = "getSizeArg = " + f.getSizeArg().toString()
+  or
+  result = "getSizeMult = " + f.getSizeMult().toString()
+  or
+  result = "getReallocPtrArg = " + f.getReallocPtrArg().toString()
+  or
+  f.requiresDealloc() and
+  result = "requiresDealloc"
+  or
+  result =
+    "getPlacementArgument = " + f.(OperatorNewAllocationFunction).getPlacementArgument().toString()
+}
+
+query predicate allocationFunctions(AllocationFunction f, string descr) {
+  descr = concat(describeAllocationFunction(f), ", ")
+}
+
+string describeAllocationExpr(AllocationExpr e) {
+  result = "getSizeExpr = " + e.getSizeExpr().toString()
+  or
+  result = "getSizeMult = " + e.getSizeMult().toString()
+  or
+  result = "getSizeBytes = " + e.getSizeBytes().toString()
+  or
+  result = "getReallocPtr = " + e.getReallocPtr().toString()
+  or
+  e.requiresDealloc() and
+  result = "requiresDealloc"
+}
+
+query predicate allocationExprs(AllocationExpr e, string descr) {
+  descr = concat(describeAllocationExpr(e), ", ")
+}
+
+string describeDeallocationFunction(DeallocationFunction f) {
+  result = "getFreedArg = " + f.getFreedArg().toString()
+}
+
+query predicate deallocationFunctions(DeallocationFunction f, string descr) {
+  descr = concat(describeDeallocationFunction(f), ", ")
+}
+
+string describeDeallocationExpr(DeallocationExpr e) {
+  result = "getFreedExpr = " + e.getFreedExpr().toString()
+}
+
+query predicate deallocationExprs(DeallocationExpr e, string descr) {
+  descr = concat(describeDeallocationExpr(e), ", ")
+}
--- a/cpp/ql/test/library-tests/allocators/placement.expected
+++ b/cpp/ql/test/library-tests/allocators/placement.expected
@@ -1,2 +0,0 @@
-| allocators.cpp:129:3:129:21 | new | allocators.cpp:129:7:129:13 | & ... |
-| allocators.cpp:132:3:132:17 | new[] | allocators.cpp:132:7:132:9 | buf |
--- a/cpp/ql/test/library-tests/allocators/placement.ql
+++ b/cpp/ql/test/library-tests/allocators/placement.ql
@@ -1,4 +0,0 @@
-import cpp
-
-from NewOrNewArrayExpr new
-select new, new.getPlacementPointer() as placement
--- a/java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.java
+++ b/java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.java
@@ -0,0 +1,8 @@
+public class TestServlet extends HttpServlet {
+    protected void doGet(HttpServletRequest request, HttpServletResponse response)
+    throws ServletException, IOException {
+        // BAD: a URL from a remote source is opened with URL#openStream()
+        URL url = new URL(request.getParameter("url"));
+        InputStream inputStream = new URL(url).openStream();
+    }
+}
--- a/java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.qhelp
+++ b/java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.qhelp
@@ -0,0 +1,33 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Calling <code>openStream</code> on URLs created from remote source can lead to local file disclosure.</p>
+<p>If <code>openStream</code> is called on a <code>java.net.URL</code>, that was created from a remote source,
+an attacker can try to pass absolute URLs starting with <code>file://</code> or <code>jar://</code> to access
+local resources in addition to remote ones.</p>
+</overview>
+
+<recommendation>
+<p>When you construct a URL using <code>java.net.URL</code> from a remote source,
+don't call <code>openStream</code> on it. Instead, use an HTTP Client to fetch the URL and access its content.
+You should also validate the URL to check that it uses the correct protocol and host combination.</p>
+</recommendation>
+
+<example>
+<p>The following example shows an URL that is constructed from a request parameter. Afterwards <code>openStream</code>
+is called on the URL, potentially leading to a local file access.</p>
+<sample src="OpenStream.java" />
+</example>
+
+<references>
+<li>Java Platform, Standard Edition 11, API Specification: 
+<a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URL.html">
+Class URL</a>.
+</li>
+<!--  LocalWords:  CWE -->
+</references>
+
+</qhelp>
--- a/java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.ql
+++ b/java/ql/src/experimental/Security/CWE/CWE-036/OpenStream.ql
@@ -0,0 +1,55 @@
+/**
+ * @name openStream called on URLs created from remote source
+ * @description Calling openStream on URLs created from remote source
+ * can lead to local file disclosure.
+ * @kind path-problem
+ */
+
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.FlowSources
+import DataFlow::PathGraph
+
+class URLConstructor extends ClassInstanceExpr {
+  URLConstructor() { this.getConstructor().getDeclaringType() instanceof TypeUrl }
+
+  Expr stringArg() {
+    // Query only in URL's that were constructed by calling the single parameter string constructor.
+    this.getConstructor().getNumberOfParameters() = 1 and
+    this.getConstructor().getParameter(0).getType() instanceof TypeString and
+    result = this.getArgument(0)
+  }
+}
+
+class URLOpenStreamMethod extends Method {
+  URLOpenStreamMethod() {
+    this.getDeclaringType() instanceof TypeUrl and
+    this.getName() = "openStream"
+  }
+}
+
+class RemoteURLToOpenStreamFlowConfig extends TaintTracking::Configuration {
+  RemoteURLToOpenStreamFlowConfig() { this = "OpenStream::RemoteURLToOpenStreamFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess m |
+      sink.asExpr() = m.getQualifier() and m.getMethod() instanceof URLOpenStreamMethod
+    )
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(URLConstructor u |
+      node1.asExpr() = u.stringArg() and
+      node2.asExpr() = u
+    )
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess call
+where
+  sink.getNode().asExpr() = call.getQualifier() and
+  any(RemoteURLToOpenStreamFlowConfig c).hasFlowPath(source, sink)
+select call, source, sink,
+  "URL on which openStream is called may have been constructed from remote source"
--- a/java/ql/src/semmle/code/java/frameworks/Networking.qll
+++ b/java/ql/src/semmle/code/java/frameworks/Networking.qll
@@ -12,6 +12,10 @@ class TypeSocket extends RefType {
  TypeSocket() { hasQualifiedName("java.net", "Socket") }
 }

+class TypeUrl extends RefType {
+  TypeUrl() { hasQualifiedName("java.net", "URL") }
+}
+
 class URLConnectionGetInputStreamMethod extends Method {
  URLConnectionGetInputStreamMethod() {
    getDeclaringType() instanceof TypeUrlConnection and
--- a/javascript/extractor/src/com/semmle/jcorn/ESNextParser.java
+++ b/javascript/extractor/src/com/semmle/jcorn/ESNextParser.java
@@ -482,6 +482,7 @@ public class ESNextParser extends JSXParser {

      if (code == '_') {
        if (underscoreAllowed) {
+          seenUnderscoreNumericSeparator = true;
          // no adjacent underscores
          underscoreAllowed = false;
          ++this.pos;
--- a/javascript/extractor/src/com/semmle/jcorn/Parser.java
+++ b/javascript/extractor/src/com/semmle/jcorn/Parser.java
@@ -145,6 +145,11 @@ public class Parser {
  private Stack<LabelInfo> labels;
  protected int yieldPos, awaitPos;

+  /**
+   * Set to true by {@link ESNextParser#readInt} if the parsed integer contains an underscore.
+   */
+  protected boolean seenUnderscoreNumericSeparator = false;
+
  /**
   * For readability purposes, we pass this instead of false as the argument to the
   * hasDeclareKeyword parameter (which only exists in TypeScript).
@@ -654,7 +659,7 @@ public class Parser {
      case 58:
        ++this.pos;
        return this.finishToken(TokenType.colon);
-      case 35: 
+      case 35:
        ++this.pos;
        return this.finishToken(TokenType.pound);
      case 63:
@@ -847,6 +852,10 @@ public class Parser {
    }

    String str = inputSubstring(start, this.pos);
+    if (seenUnderscoreNumericSeparator) {
+      str = str.replace("_", "");
+      seenUnderscoreNumericSeparator = false;
+    }
    Number val = null;
    if (isFloat) val = parseFloat(str);
    else if (!octal || str.length() == 1) val = parseInt(str, 10);
--- a/javascript/extractor/src/com/semmle/js/extractor/Main.java
+++ b/javascript/extractor/src/com/semmle/js/extractor/Main.java
@@ -38,7 +38,7 @@ public class Main {
   * A version identifier that should be updated every time the extractor changes in such a way that
   * it may produce different tuples for the same file under the same {@link ExtractorConfig}.
   */
-  public static final String EXTRACTOR_VERSION = "2020-02-14";
+  public static final String EXTRACTOR_VERSION = "2020-04-01";

  public static final Pattern NEWLINE = Pattern.compile("\n");

--- a/javascript/ql/test/library-tests/Constants/Constants.expected
+++ b/javascript/ql/test/library-tests/Constants/Constants.expected
@@ -1,3 +1,123 @@
+getIntValue
+| tst2.ts:1:21:1:21 | 1 | 1 |
+| tst.js:6:1:6:1 | 1 | 1 |
+| tst.js:11:1:11:2 | -1 | -1 |
+| tst.js:11:2:11:2 | 1 | 1 |
+| tst.js:12:2:12:2 | 0 | 0 |
+| tst.js:26:3:26:3 | 0 | 0 |
+| tst.js:29:6:29:6 | 0 | 0 |
+| tst.js:35:1:35:1 | 1 | 1 |
+| tst.js:35:5:35:5 | 2 | 2 |
+| tst.js:35:9:35:9 | 3 | 3 |
+| tst.js:37:1:37:3 | (1) | 1 |
+| tst.js:37:2:37:2 | 1 | 1 |
+| tst.js:39:4:39:4 | 1 | 1 |
+| tst.js:40:1:40:1 | 1 | 1 |
+| tst.js:42:1:42:1 | 1 | 1 |
+| tst.js:42:4:42:4 | 2 | 2 |
+| tst.js:42:7:42:7 | 3 | 3 |
+| tst.js:43:4:43:4 | 2 | 2 |
+| tst.js:43:7:43:7 | 3 | 3 |
+| tst.js:44:1:44:1 | 1 | 1 |
+| tst.js:44:7:44:7 | 3 | 3 |
+| tst.js:45:1:45:1 | 1 | 1 |
+| tst.js:45:4:45:4 | 2 | 2 |
+| tst.js:47:5:47:5 | 1 | 1 |
+| tst.js:48:7:48:7 | 1 | 1 |
+| tst.js:49:6:49:6 | 1 | 1 |
+| tst.js:52:5:52:9 | 1_000 | 1000 |
+| tst.js:53:5:53:13 | 1_000_123 | 1000123 |
+| tst.js:54:5:54:17 | 1_000_000_000 | 1000000000 |
+| tst.js:56:5:56:10 | 123_00 | 12300 |
+| tst.js:57:5:57:10 | 12_300 | 12300 |
+| tst.js:58:5:58:12 | 12345_00 | 1234500 |
+| tst.js:59:5:59:12 | 123_4500 | 1234500 |
+| tst.js:60:5:60:13 | 1_234_500 | 1234500 |
+| tst.js:62:5:62:14 | 0xaa_bb_cc | 11189196 |
+getFloatValue
+| tst2.ts:1:21:1:21 | 1 | 1.0 |
+| tst.js:6:1:6:1 | 1 | 1.0 |
+| tst.js:11:2:11:2 | 1 | 1.0 |
+| tst.js:12:2:12:2 | 0 | 0.0 |
+| tst.js:26:3:26:3 | 0 | 0.0 |
+| tst.js:29:6:29:6 | 0 | 0.0 |
+| tst.js:35:1:35:1 | 1 | 1.0 |
+| tst.js:35:5:35:5 | 2 | 2.0 |
+| tst.js:35:9:35:9 | 3 | 3.0 |
+| tst.js:37:2:37:2 | 1 | 1.0 |
+| tst.js:39:4:39:4 | 1 | 1.0 |
+| tst.js:40:1:40:1 | 1 | 1.0 |
+| tst.js:42:1:42:1 | 1 | 1.0 |
+| tst.js:42:4:42:4 | 2 | 2.0 |
+| tst.js:42:7:42:7 | 3 | 3.0 |
+| tst.js:43:4:43:4 | 2 | 2.0 |
+| tst.js:43:7:43:7 | 3 | 3.0 |
+| tst.js:44:1:44:1 | 1 | 1.0 |
+| tst.js:44:7:44:7 | 3 | 3.0 |
+| tst.js:45:1:45:1 | 1 | 1.0 |
+| tst.js:45:4:45:4 | 2 | 2.0 |
+| tst.js:47:5:47:5 | 1 | 1.0 |
+| tst.js:48:7:48:7 | 1 | 1.0 |
+| tst.js:49:6:49:6 | 1 | 1.0 |
+| tst.js:52:5:52:9 | 1_000 | 1000.0 |
+| tst.js:53:5:53:13 | 1_000_123 | 1000123.0 |
+| tst.js:54:5:54:17 | 1_000_000_000 | 1.0E9 |
+| tst.js:55:5:55:18 | 101_475_938.38 | 1.0147593838E8 |
+| tst.js:56:5:56:10 | 123_00 | 12300.0 |
+| tst.js:57:5:57:10 | 12_300 | 12300.0 |
+| tst.js:58:5:58:12 | 12345_00 | 1234500.0 |
+| tst.js:59:5:59:12 | 123_4500 | 1234500.0 |
+| tst.js:60:5:60:13 | 1_234_500 | 1234500.0 |
+| tst.js:61:5:61:10 | 1e1_00 | 1.0E100 |
+| tst.js:62:5:62:14 | 0xaa_bb_cc | 1.1189196E7 |
+getLiteralValue
+| tst2.ts:1:21:1:21 | 1 | 1 |
+| tst.js:1:1:1:3 | "a" | a |
+| tst.js:2:1:2:3 | "b" | b |
+| tst.js:2:7:2:9 | "c" | c |
+| tst.js:3:1:3:3 | "d" | d |
+| tst.js:3:7:3:9 | "e" | e |
+| tst.js:3:13:3:15 | "f" | f |
+| tst.js:6:1:6:1 | 1 | 1 |
+| tst.js:8:1:8:5 | false | false |
+| tst.js:9:1:9:4 | true | true |
+| tst.js:11:2:11:2 | 1 | 1 |
+| tst.js:12:2:12:2 | 0 | 0 |
+| tst.js:14:1:14:4 | null | null |
+| tst.js:20:1:20:3 | /x/ | /x/ |
+| tst.js:24:5:24:7 | "x" | x |
+| tst.js:26:3:26:3 | 0 | 0 |
+| tst.js:29:6:29:6 | 0 | 0 |
+| tst.js:35:1:35:1 | 1 | 1 |
+| tst.js:35:5:35:5 | 2 | 2 |
+| tst.js:35:9:35:9 | 3 | 3 |
+| tst.js:37:2:37:2 | 1 | 1 |
+| tst.js:39:4:39:4 | 1 | 1 |
+| tst.js:40:1:40:1 | 1 | 1 |
+| tst.js:42:1:42:1 | 1 | 1 |
+| tst.js:42:4:42:4 | 2 | 2 |
+| tst.js:42:7:42:7 | 3 | 3 |
+| tst.js:43:4:43:4 | 2 | 2 |
+| tst.js:43:7:43:7 | 3 | 3 |
+| tst.js:44:1:44:1 | 1 | 1 |
+| tst.js:44:7:44:7 | 3 | 3 |
+| tst.js:45:1:45:1 | 1 | 1 |
+| tst.js:45:4:45:4 | 2 | 2 |
+| tst.js:47:5:47:5 | 1 | 1 |
+| tst.js:48:7:48:7 | 1 | 1 |
+| tst.js:49:6:49:6 | 1 | 1 |
+| tst.js:52:5:52:9 | 1_000 | 1000 |
+| tst.js:53:5:53:13 | 1_000_123 | 1000123 |
+| tst.js:54:5:54:17 | 1_000_000_000 | 1000000000 |
+| tst.js:55:5:55:18 | 101_475_938.38 | 1.0147593838E8 |
+| tst.js:56:5:56:10 | 123_00 | 12300 |
+| tst.js:57:5:57:10 | 12_300 | 12300 |
+| tst.js:58:5:58:12 | 12345_00 | 1234500 |
+| tst.js:59:5:59:12 | 123_4500 | 1234500 |
+| tst.js:60:5:60:13 | 1_234_500 | 1234500 |
+| tst.js:61:5:61:10 | 1e1_00 | 1.0E100 |
+| tst.js:62:5:62:14 | 0xaa_bb_cc | 11189196 |
+#select
 | tst2.ts:1:13:1:21 | <number>1 |
 | tst2.ts:1:21:1:21 | 1 |
 | tst.js:1:1:1:3 | "a" |
@@ -61,3 +181,25 @@
 | tst.js:48:1:48:7 | x.p = 1 |
 | tst.js:48:7:48:7 | 1 |
 | tst.js:49:6:49:6 | 1 |
+| tst.js:52:1:52:9 | x = 1_000 |
+| tst.js:52:5:52:9 | 1_000 |
+| tst.js:53:1:53:13 | x = 1_000_123 |
+| tst.js:53:5:53:13 | 1_000_123 |
+| tst.js:54:1:54:17 | x = 1_000_000_000 |
+| tst.js:54:5:54:17 | 1_000_000_000 |
+| tst.js:55:1:55:18 | x = 101_475_938.38 |
+| tst.js:55:5:55:18 | 101_475_938.38 |
+| tst.js:56:1:56:10 | x = 123_00 |
+| tst.js:56:5:56:10 | 123_00 |
+| tst.js:57:1:57:10 | x = 12_300 |
+| tst.js:57:5:57:10 | 12_300 |
+| tst.js:58:1:58:12 | x = 12345_00 |
+| tst.js:58:5:58:12 | 12345_00 |
+| tst.js:59:1:59:12 | x = 123_4500 |
+| tst.js:59:5:59:12 | 123_4500 |
+| tst.js:60:1:60:13 | x = 1_234_500 |
+| tst.js:60:5:60:13 | 1_234_500 |
+| tst.js:61:1:61:10 | x = 1e1_00 |
+| tst.js:61:5:61:10 | 1e1_00 |
+| tst.js:62:1:62:14 | x = 0xaa_bb_cc |
+| tst.js:62:5:62:14 | 0xaa_bb_cc |
--- a/javascript/ql/test/library-tests/Constants/Constants.ql
+++ b/javascript/ql/test/library-tests/Constants/Constants.ql
@@ -2,3 +2,9 @@ import javascript

 from ConstantExpr c
 select c
+
+query int getIntValue(Expr e) { result = e.getIntValue() }
+
+query float getFloatValue(NumberLiteral e) { result = e.getFloatValue() }
+
+query string getLiteralValue(Literal lit) { result = lit.getValue() }
--- a/javascript/ql/test/library-tests/Constants/tst.js
+++ b/javascript/ql/test/library-tests/Constants/tst.js
@@ -48,3 +48,15 @@ x = 1;
 x.p = 1;
 x += 1;
 x += x;
+
+x = 1_000;
+x = 1_000_123;
+x = 1_000_000_000;
+x = 101_475_938.38;
+x = 123_00;
+x = 12_300;
+x = 12345_00;
+x = 123_4500;
+x = 1_234_500;
+x = 1e1_00;
+x = 0xaa_bb_cc;
--- a/javascript/ql/test/library-tests/RangeAnalysis/constants.js
+++ b/javascript/ql/test/library-tests/RangeAnalysis/constants.js
@@ -6,3 +6,8 @@ function f() {
  if (two > one) {}  // NOT OK - always true
  if (two <= one) {} // NOT OK - always false
 }
+
+function underscores(x) {
+  if (x >= 1_000_000) return; // OK
+  if (x >= 1_000) return; // OK
+}
--- a/python/ql/src/Functions/ReturnValueIgnored.ql
+++ b/python/ql/src/Functions/ReturnValueIgnored.ql
@@ -15,17 +15,18 @@
 */

 import python
+import semmle.python.objects.Callables

 predicate meaningful_return_value(Expr val) {
    val instanceof Name
    or
    val instanceof BooleanLiteral
    or
-    exists(FunctionObject callee |
+    exists(FunctionValue callee |
        val = callee.getACall().getNode() and returns_meaningful_value(callee)
    )
    or
-    not exists(FunctionObject callee | val = callee.getACall().getNode()) and not val instanceof Name
+    not exists(FunctionValue callee | val = callee.getACall().getNode()) and not val instanceof Name
 }

 /* Value is used before returning, and thus its value is not lost if ignored */
@@ -35,10 +36,10 @@ predicate used_value(Expr val) {
    )
 }

-predicate returns_meaningful_value(FunctionObject f) {
-    not exists(f.getFunction().getFallthroughNode()) and
+predicate returns_meaningful_value(FunctionValue f) {
+    not exists(f.getScope().getFallthroughNode()) and
    (
-        exists(Return ret, Expr val | ret.getScope() = f.getFunction() and val = ret.getValue() |
+        exists(Return ret, Expr val | ret.getScope() = f.getScope() and val = ret.getValue() |
            meaningful_return_value(val) and
            not used_value(val)
        )
@@ -48,7 +49,9 @@ predicate returns_meaningful_value(FunctionObject f) {
         * Ignore __import__ as it is often called purely for side effects
         */

-        f.isC() and f.getAnInferredReturnType() != theNoneType() and not f.getName() = "__import__"
+        f.isBuiltin() and
+        f.getAnInferredReturnType() != ClassValue::nonetype() and
+        not f.getName() = "__import__"
    )
 }

@@ -61,7 +64,7 @@ predicate wrapped_in_try_except(ExprStmt call) {
    )
 }

-from ExprStmt call, FunctionObject callee, float percentage_used, int total
+from ExprStmt call, FunctionValue callee, float percentage_used, int total
 where
    call.getValue() = callee.getACall().getNode() and
    returns_meaningful_value(callee) and
--- a/python/ql/src/analysis/ImportFailure.qhelp
+++ b/python/ql/src/analysis/ImportFailure.qhelp
@@ -21,7 +21,7 @@ Ensure that all required modules and packages can be found when running the extr
 </recommendation>
 <references>

-<li>Semmle Tutorial: <a href="https://semmle.com/wiki/pages/viewpage.action?pageId=9493108">Basic project creation (Python)</a>.</li>
+<li>Semmle Tutorial: <a href="https://help.semmle.com/codeql/codeql-cli/procedures/create-codeql-database.html">Creating a CodeQL database</a>.</li>


 </references>
--- a/python/ql/src/semmle/python/objects/ObjectAPI.qll
+++ b/python/ql/src/semmle/python/objects/ObjectAPI.qll
@@ -581,6 +581,13 @@ abstract class FunctionValue extends CallableValue {
            exists(Expr expr, AstNode origin | expr.pointsTo(this, origin) | not origin instanceof Lambda)
        )
    }
+
+    /** Gets a class that this function may return */
+    ClassValue getAnInferredReturnType() {
+        result = TBuiltinClassObject(this.(BuiltinFunctionObjectInternal).getReturnType())
+        or
+        result = TBuiltinClassObject(this.(BuiltinMethodObjectInternal).getReturnType())
+    }
 }

 /** Class representing Python functions */
--- a/python/ql/test/query-tests/Functions/return_values/ReturnValueIgnored.expected
+++ b/python/ql/test/query-tests/Functions/return_values/ReturnValueIgnored.expected
@@ -1,3 +1,3 @@
 | functions_test.py:159:5:159:9 | ExprStmt | Call discards return value of function $@. The result is used in 80% of calls. | functions_test.py:12:1:12:11 | Function ok2 | ok2 |
 | functions_test.py:160:5:160:9 | ExprStmt | Call discards return value of function $@. The result is used in 80% of calls. | functions_test.py:36:1:36:11 | Function ok4 | ok4 |
-| functions_test.py:161:5:161:17 | ExprStmt | Call discards return value of function $@. The result is used in 80% of calls. | file://:Compiled Code:0:0:0:0 | Builtin-function sorted | sorted |
+| functions_test.py:161:5:161:17 | ExprStmt | Call discards return value of function $@. The result is used in 80% of calls. | file://:0:0:0:0 | Builtin-function sorted | sorted |