Merge pull request #674 from hvitved/csharp/cache-get-label

C#: Cache `NamedElement::getLabel()`

Author: calumgrant

csharp/ql/src/Likely Bugs/Collections/ContainerLengthCmpOffByOne.ql

@@ -20,10 +20,10 @@ import semmle.code.csharp.commons.ComparisonTest
 class IndexGuard extends ComparisonTest {
   VariableAccess indexAccess;
   Variable array;
-  
+
   IndexGuard() {
     this.getFirstArgument() = indexAccess and
-    this.getSecondArgument() = any(PropertyAccess lengthAccess | 
+    this.getSecondArgument() = any(PropertyAccess lengthAccess |
       lengthAccess.getQualifier() = array.getAnAccess() and
       lengthAccess.getTarget().hasName("Length")
     )
@@ -50,7 +50,7 @@ from IndexGuard incorrectGuard, Variable array, Variable index, ElementAccess ea
 where
   // Look for `index <= array.Length` or `array.Length >= index`
   incorrectGuard.controls(array, index) and
-  incorrectGuard.isIncorrect() and 
+  incorrectGuard.isIncorrect() and
   // Look for `array[index]`
   ea.getQualifier() = array.getAnAccess() and
   ea.getIndex(0) = indexAccess and

csharp/ql/src/Security Features/CWE-730/ReDoS.ql

@@ -19,7 +19,7 @@ from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode
 where
   c.hasFlowPath(source, sink) and
   // No global timeout set
-  not exists(RegexGlobalTimeout r) and 
+  not exists(RegexGlobalTimeout r) and
   (
     sink.getNode() instanceof Sink
     or

csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll

@@ -1,6 +1,6 @@
 /**
  * Provides a list of NuGet packages with known vulnerabilities.
- * 
+ *
  * To add a new vulnerability follow the existing pattern.
  * Create a new class that extends the abstract class `Vulnerability`,
  * supplying the name and the URL, and override one (or both) of
@@ -73,9 +73,9 @@ class MicrosoftAdvisory4021279 extends Vulnerability {
 
 class CVE_2017_8700 extends Vulnerability {
   CVE_2017_8700() { this = "CVE-2017-8700" }
-  
+
   override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/279" }
-  
+
   override predicate matchesRange(string name, Version affected, Version fixed) {
     (
       name = "Microsoft.AspNetCore.Mvc.Core"
@@ -91,9 +91,9 @@ class CVE_2017_8700 extends Vulnerability {
 
 class CVE_2018_0765 extends Vulnerability {
   CVE_2018_0765() { this = "CVE-2018-0765" }
-  
+
   override string getUrl() { result = "https://github.com/dotnet/announcements/issues/67" }
-  
+
   override predicate matchesRange(string name, Version affected, Version fixed) {
     name = "System.Security.Cryptography.Xml" and
     affected = "0.0.0" and
@@ -103,7 +103,7 @@ class CVE_2018_0765 extends Vulnerability {
 
 class AspNetCore_Mar18 extends Vulnerability {
   AspNetCore_Mar18() { this = "ASPNETCore-Mar18" }
-  
+
   override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/300" }
 
   override predicate matchesRange(string name, Version affected, Version fixed) {
@@ -125,9 +125,9 @@ class AspNetCore_Mar18 extends Vulnerability {
 
 class CVE_2018_8409 extends Vulnerability {
   CVE_2018_8409() { this = "CVE-2018-8409" }
-  
+
   override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/316" }
-  
+
   override predicate matchesRange(string name, Version affected, Version fixed) {
     name = "System.IO.Pipelines" and affected = "4.5.0" and fixed = "4.5.1"
     or
@@ -138,9 +138,9 @@ class CVE_2018_8409 extends Vulnerability {
 
 class CVE_2018_8171 extends Vulnerability {
   CVE_2018_8171() { this = "CVE-2018-8171" }
-  
+
   override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/310" }
-  
+
   override predicate matchesRange(string name, Version affected, Version fixed) {
     name = "Microsoft.AspNetCore.Identity" and (
       affected = "1.0.0" and fixed = "1.0.6"
@@ -204,7 +204,7 @@ class CVE_2018_8356 extends Vulnerability {
 
 class ASPNETCore_Jul18 extends Vulnerability {
   ASPNETCore_Jul18() { this = "ASPNETCore-July18" }
-  
+
   override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/311" }
 
   override predicate matchesRange(string name, Version affected, Version fixed) {

csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll

@@ -286,7 +286,7 @@ class DereferenceableExpr extends Expr {
           ie = any(IsTypeExpr ite | ite.getCheckedType() = ite.getExpr().getType()) and
           branch = false and
           isNull = true
-        ) 
+        )
       )
       or
       this.hasNullableType() and
@@ -1189,7 +1189,7 @@ module Internal {
         g1 = cond and
         v1 = v.getDualValue() and
         (
-          // g1 === g2 ? e : ...; 
+          // g1 === g2 ? e : ...;
           g2 = cond.getCondition() and
           v2 = TBooleanValue(branch.booleanNot())
           or

csharp/ql/src/semmle/code/csharp/dataflow/DataFlow.qll

@@ -90,7 +90,7 @@ module DataFlow {
     localFlowStep*(source, sink)
   }
 
-  predicate localFlowStep = localFlowStepCached/2;
+  predicate localFlowStep = Internal::LocalFlow::step/2;
 
   /**
    * A data flow node augmented with a call context and a configuration. Only
@@ -690,12 +690,14 @@ module DataFlow {
     /**
      * Provides predicates related to local data flow.
      */
-    private module LocalFlow {
+    module LocalFlow {
       /**
        * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
        * (intra-procedural) step.
        */
-      predicate localFlowStepNonCached(Node nodeFrom, Node nodeTo) {
+      cached
+      predicate step(Node nodeFrom, Node nodeTo) {
+        forceCachingInSameStage() and
         localFlowStepExpr(nodeFrom.asExpr(), nodeTo.asExpr())
         or
         // Flow from source to SSA definition
@@ -1119,6 +1121,8 @@ module DataFlow {
      * same stage.
      */
     cached module Cached {
+      cached predicate forceCachingInSameStage() { any() }
+
       cached newtype TNode =
         TExprNode(DotNet::Expr e)
         or
@@ -1137,14 +1141,6 @@ module DataFlow {
           )
         }
 
-      /**
-       * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
-       * (intra-procedural) step.
-       */
-      cached predicate localFlowStepCached(Node nodeFrom, Node nodeTo) {
-        LocalFlow::localFlowStepNonCached(nodeFrom, nodeTo)
-      }
-
       /**
        * Holds if `pred` can flow to `succ`, by jumping from one callable to
        * another.

csharp/ql/src/semmle/code/dotnet/Element.qll

@@ -80,6 +80,7 @@ class NamedElement extends Element, @dotnet_named_element {
   }
 
   /** Gets a unique string label for this element. */
+  cached
   string getLabel() { none() }
 
   /**

csharp/ql/test/library-tests/cil/regressions/Methods.cs

@@ -3,7 +3,7 @@
  * This tests the correct extraction of F<T>, and we should end up with
  * 2 constructed methods of F<T>.
  */
- 
+
 // semmle-extractor-options: --cil
 
 namespace Methods

csharp/ql/test/query-tests/Likely Bugs/Collections/ContainerLengthCmpOffByOne/ContainerLengthCmpOffByOne.cs

@@ -48,7 +48,7 @@ void Test2(string[] args)
     }
 
     void Test3(string[] args)
-    {    
+    {
         // GOOD: Guarded by ternary operator.
         for (int i = 0; i <= args.Length; i++)
         {
@@ -68,7 +68,7 @@ void Test4(string[] args)
     }
 
     void Test5(string[] args)
-    {  
+    {
         // GOOD: A valid test of Length.
         for (int i = 0; i != args.Length; i++)
         {
@@ -94,6 +94,6 @@ void Test7(string[] args)
         for (int i = 0; i <= args.Length; i++)
         {
             bool b = i == args.Length || args[i] == "x";
-        }        
+        }
     }
 }

csharp/ql/test/query-tests/Security Features/CWE-937/csproj.config

@@ -5,12 +5,12 @@
     <PackageReference Include="System.Net.Http" Version="4.3.4" />
     <PackageReference Include="System.Text.Encodings.Web" Version="4.2.9" />
     <PackageReference Include="System.Text.Encodings.Web" Version="4.3.1" />
-    
+
     <!-- These are BAD -->
     <PackageReference Include="System.Text.Encodings.Web" Version="4.3.0" />
     <PackageReference Include="system.text.encodings.web" Version="4.3" />
     <PackageReference Include="System.Net.Http" Version="4.1.1" />
     <PackageReference Include="System.Net.Http" Version="4.1.2" />
-    
+
   </ItemGroup>
 </Project>

csharp/ql/test/query-tests/Security Features/CWE-937/packages.config

@@ -4,7 +4,7 @@
   <package id="System.IO.Pipelines" version="4.5.1" targetFramework="net45" />
   <package id="System.IO.Pipelines" version="4.5.1.0" targetFramework="net45" />
   <package id="Microsoft.AspNetCore.All" version="2.0.9" targetFramework="net45" />
-  
+
   <!-- These are BAD -->
   <package id="System.IO.Pipelines" version="4.5.0" targetFramework="net45" />
   <package id="System.IO.Pipelines" version="4.5.0.0" targetFramework="net45" />