Merge pull request #1518 from Semmle/rc/1.21

Merge rc/1.21 into master

Author: yh-semmle

change-notes/1.21/analysis-csharp.md

@@ -4,12 +4,18 @@
 
 C# analysis now supports the extraction and analysis of many C# 8 features. For details see [Changes to code extraction](#changes-to-code-extraction) and [Changes to QL libraries](#changes-to-ql-libraries) below.
 
+## New queries
+
+| **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |
+|-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| Thread-unsafe capturing of an ICryptoTransform object (`cs/thread-unsafe-icryptotransform-captured-in-lambda`) | concurrency, security, external/cwe/cwe-362 | Highlights instances of classes where a field of type `System.Security.Cryptography.ICryptoTransform` is captured by a lambda, and appears to be used in a thread initialization method. Results are not shown on [LGTM](https://lgtm.com/rules/1508141845995/) by default. |
+
 ## Changes to existing queries
 
 | **Query**                    | **Expected impact**    | **Change**                        |
 |------------------------------|------------------------|-----------------------------------|
-| Class defines a field that uses an ICryptoTransform class in a way that would be unsafe for concurrent threads (`cs/thread-unsafe-icryptotransform-field-in-class`) | Fewer false positive results | The criteria for a result has changed to include nested properties, nested fields, and collections. The format of the alert message has changed to highlight the static field. |
 | Constant condition (`cs/constant-condition`) | Fewer false positive results | The query now ignores code where the `null` value is in a conditional expression on the left hand side of a null-coalescing expression. For example, in `(a ? b : null) ?? c`, `null` is not considered to be a constant condition. |
+| Thread-unsafe use of a static ICryptoTransform field (`cs/thread-unsafe-icryptotransform-field-in-class`) | Fewer false positive results | The criteria for a result has changed to include nested properties, nested fields, and collections. The format of the alert message has changed to highlight the static field. The query name has been updated. |
 | Useless upcast (`cs/useless-upcast`) | Fewer false positive results | The query now ignores code where the upcast is used to disambiguate the target of a constructor call. |
 
 ## Changes to code extraction

change-notes/1.21/analysis-python.md

@@ -36,6 +36,7 @@ We welcome feedback on the new implementation, particularly any surprising chang
 | **Query** | **Tags** | **Purpose** |
 |-----------|----------|-------------|
 | Accepting unknown SSH host keys when using Paramiko (`py/paramiko-missing-host-key-validation`) | security, external/cwe/cwe-295 | Finds instances where Paramiko is configured to accept unknown host keys. Results are shown [on LGTM](https://lgtm.com/rules/1508297729270/) by default. |
+| Pythagorean calculation with sub-optimal numerics (`py/pythagorean`) | accuracy | Finds instances of hypotenuse calculation using `math.sqrt` instead of `math.hypot`. Results are not shown on LGTM by default. |
 | Use of 'return' or 'yield' outside a function (`py/return-or-yield-outside-function`) | reliability, correctness | Finds instances where `return`, `yield`, and `yield from` are used outside a function. Results are not shown on LGTM by default. |
 
 ## Changes to code extraction

csharp/ql/src/Likely Bugs/ThreadUnsafeICryptoTransform.ql

@@ -1,5 +1,5 @@
 /**
- * @name Class defines a field that uses an ICryptoTransform class in a way that would be unsafe for concurrent threads
+ * @name Thread-unsafe use of a static ICryptoTransform field
  * @description The class has a field that directly or indirectly make use of a static System.Security.Cryptography.ICryptoTransform object.
  *              Using this an instance of this class in concurrent threads is dangerous as it may not only result in an error,
  *              but under some circumstances may also result in incorrect results.

csharp/ql/src/Likely Bugs/ThreadUnsafeICryptoTransformLambda.ql

@@ -1,5 +1,5 @@
 /**
- * @name Potential usage of an object implementing ICryptoTransform class in a way that would be unsafe for concurrent threads.
+ * @name Thread-unsafe capturing of an ICryptoTransform object
  * @description An instance of a class that either implements or has a field of type System.Security.Cryptography.ICryptoTransform is being captured by a lambda,
  *              and used in what seems to be a thread initialization method.
  *              Using an instance of this class in concurrent threads is dangerous as it may not only result in an error,

python/ql/src/semmle/python/objects/Modules.qll

@@ -182,13 +182,8 @@ class PackageObjectInternal extends ModuleObjectInternal, TPackageObject {
         or
         exists(Module init |
             init = this.getSourceModule() and
-            (
-                /* There is no variable shadowing the name of the child module */
-                not exists(EssaVariable var | var.getAUse() = init.getANormalExit() and var.getSourceVariable().getName() = name)
-                or
-                /* The variable shadowing the name of the child module is undefined at exit */
-                ModuleAttributes::pointsToAtExit(init, name, ObjectInternal::undefined(), _)
-            ) and
+            /* The variable shadowing the name of the child module is undefined at exit */
+            ModuleAttributes::pointsToAtExit(init, name, ObjectInternal::undefined(), _) and
             not name = "__init__" and
             value = this.submodule(name) and
             origin = CfgOrigin::fromObject(value)
@@ -254,6 +249,7 @@ class PythonModuleObjectInternal extends ModuleObjectInternal, TPythonModule {
     }
 
     pragma [noinline] override predicate attribute(string name, ObjectInternal value, CfgOrigin origin) {
+        value != ObjectInternal::undefined() and
         ModuleAttributes::pointsToAtExit(this.getSourceModule(), name, value, origin)
     }
 

python/ql/test/library-tests/PointsTo/import_star/Values.expected

@@ -0,0 +1,4 @@
+| nested/__init__.py:1:6:1:12 | ControlFlowNode for ImportExpr | import | nested/nested.py:0:0:0:0 | Module nested.nested |
+| nested/nested.py:1:1:1:13 | ControlFlowNode for FunctionExpr | import | nested/nested.py:1:1:1:13 | Function nested |
+| test.py:1:6:1:11 | ControlFlowNode for ImportExpr | import | file://:0:0:0:0 | Package nested |
+| test.py:2:1:2:6 | ControlFlowNode for nested | import | nested/nested.py:1:1:1:13 | Function nested |

python/ql/test/library-tests/PointsTo/import_star/Values.ql

@@ -0,0 +1,7 @@
+
+import python
+
+from ControlFlowNode f, Context ctx, Value v, ControlFlowNode origin
+where
+  f.pointsTo(ctx, v, origin)
+select f, ctx, v

python/ql/test/library-tests/PointsTo/import_star/nested/__init__.py

@@ -0,0 +1 @@
+from .nested import *

python/ql/test/library-tests/PointsTo/import_star/nested/nested.py

@@ -0,0 +1,2 @@
+def nested():
+    pass

python/ql/test/library-tests/PointsTo/import_star/test.py

@@ -0,0 +1,2 @@
+from nested import *
+nested