Merge pull request #990 from forcedotcom/dev

RELEASE: @W-12600174@: Merging `dev` to `release` for 3.10.0

Author: rmohan20

messages/run-dfa.js

@@ -1,12 +1,13 @@
 {
 	"name": "@salesforce/sfdx-scanner",
 	"description": "Static code scanner that applies quality and security rules to Apex code, and provides feedback.",
-	"version": "3.9.0",
+	"version": "3.10.0",
 	"author": "ISV SWAT",
 	"bugs": "https://github.com/forcedotcom/sfdx-scanner/issues",
 	"dependencies": {
 		"@babel/core": "^7.11.0",
 		"@babel/eslint-parser": "^7",
+		"@eslint/js": "^8.35.0",
 		"@lwc/eslint-plugin-lwc": "^1.1.2",
 		"@oclif/command": "^1",
 		"@oclif/config": "^1",
@@ -19,7 +20,7 @@
 		"@typescript-eslint/parser": "^5.14.0",
 		"cross-spawn": "^7.0.3",
 		"csv-stringify": "^6.0.5",
-		"eslint": "^8.10.0",
+		"eslint": "^8.35.0",
 		"eslint-plugin-import": "^2.25.4",
 		"eslint-plugin-jest": "^26.1.1",
 		"find-java-home": "1.2.2",
@@ -75,7 +76,7 @@
 		"@types/uuid": "^8.3.4",
 		"chai": "^4",
 		"cross-env": "^7.0.3",
-		"eslint": "^8.10.0",
+		"eslint": "^8.35.0",
 		"mocha": "^9",
 		"mocha-junit-reporter": "^2.0.0",
 		"nyc": "^15.0.0",

package.json

@@ -9,7 +9,7 @@ group = "sfdx"
 version = "1.0"
 
 val distDir = "$buildDir/../../dist"
-val pmdVersion = "6.53.0"
+val pmdVersion = "6.54.0"
 val pmdFile = "pmd-bin-$pmdVersion.zip"
 val pmdUrl = "https://github.com/pmd/pmd/releases/download/pmd_releases%2F${pmdVersion}/${pmdFile}"
 val skippableJarRegexes = setOf("""^common_[\d\.-]*\.jar""".toRegex(),

pmd-cataloger/build.gradle.kts

@@ -2834,7 +2834,7 @@
       {
         "below": "1.12.1",
         "atOrAbove": "1.3.2",
-        "severity": "High",
+        "severity": "high",
         "identifiers": {
           "summary": " vulnerable to Arbitrary Code Injection via the template function",
           "CVE": [
@@ -3836,6 +3836,34 @@
       ]
     }
   },
+  "chart.js": {
+    "vulnerabilities": [
+      {
+        "below": "2.9.4",
+        "severity": "high",
+        "identifiers": {
+          "summary": "Prototype pollution in chart.js",
+          "CVE": [
+            "CVE-2020-7746"
+          ]
+        },
+        "info": [
+          "https://github.com/advisories/GHSA-h68q-55jf-x68w"
+        ]
+      }
+    ],
+    "extractors": {
+      "uri": [
+        "/Chart.js/(§§version§§)/chart(\\.min)?\\.js",
+        "/Chart.js/(§§version§§)/Chart.bundle(\\.min)?\\.js"
+      ],
+      "filecontent": [
+        "var version=\"(§§version§§)\";const KNOWN_POSITIONS=\\[\"top\",\"bottom\",\"left\",\"right\",\"chartArea\"\\]",
+        "/\\*![\\s]+\\* Chart.js v(§§version§§)",
+        "/\\*![\\s]+\\* Chart.js[\\s]+\\* http://chartjs.org/[\\s]+\\* Version: (§§version§§)"
+      ]
+    }
+  },
   "dont check": {
     "extractors": {
       "uri": [

retire-js/RetireJsVulns.json

@@ -1,6 +1,7 @@
 package com.salesforce.config;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.salesforce.graph.ops.registry.RegistryDataLimitCalculator;
 import java.util.concurrent.TimeUnit;
 
 public final class EnvUtil {
@@ -11,6 +12,8 @@ public final class EnvUtil {
     private static final String ENV_IGNORE_PARSE_ERRORS = "SFGE_IGNORE_PARSE_ERRORS";
     private static final String ENV_LOG_WARNINGS_ON_VERBOSE = "SFGE_LOG_WARNINGS_ON_VERBOSE";
     private static final String ENV_PROGRESS_INCREMENTS = "SFGE_PROGRESS_INCREMENTS";
+    private static final String ENV_STACK_DEPTH_LIMIT = "SFGE_STACK_DEPTH_LIMIT";
+    private static final String ENV_PATH_EXPANSION_LIMIT = "SFGE_PATH_EXPANSION_LIMIT";
 
     // TODO: These should move to SfgeConfigImpl and this class should return Optionals
     @VisibleForTesting
@@ -22,10 +25,16 @@ public final class EnvUtil {
             TimeUnit.MILLISECONDS.convert(15, TimeUnit.MINUTES);
 
     @VisibleForTesting static final boolean DEFAULT_RULE_DISABLE_WARNING_VIOLATION = false;
-    @VisibleForTesting static final boolean DEFAULT_IGNORE_PARSE_ERRORS = false;
     @VisibleForTesting static final boolean DEFAULT_LOG_WARNINGS_ON_VERBOSE = false;
     @VisibleForTesting static final int DEFAULT_PROGRESS_INCREMENTS = 10;
 
+    /** Artificial stack depth limit to keep path expansion under control. */
+    @VisibleForTesting static final int DEFAULT_STACK_DEPTH_LIMIT = 450;
+
+    @VisibleForTesting
+    static final int DEFAULT_PATH_EXPANSION_LIMIT =
+            RegistryDataLimitCalculator.getApexPathExpanderRegistryLimit();
+
     /**
      * Returns the value of the {@link #ENV_RULE_THREAD_COUNT} environment variable if set, else
      * {@link #DEFAULT_RULE_THREAD_COUNT}. Should be used to set the number of threads that can be
@@ -76,6 +85,22 @@ static int getProgressIncrements() {
         return getIntOrDefault(ENV_PROGRESS_INCREMENTS, DEFAULT_PROGRESS_INCREMENTS);
     }
 
+    /**
+     * Returns stack depth limit upto which Graph Engine attempts to dig. Depths beyond this have a
+     * high probability of throwing a StackOverFlow exception.
+     */
+    static int getStackDepthLimit() {
+        return getIntOrDefault(ENV_STACK_DEPTH_LIMIT, DEFAULT_STACK_DEPTH_LIMIT);
+    }
+
+    /**
+     * @return Maximum limit that items in registry can reach. Checks for env variable and returns
+     *     default when not set.
+     */
+    static int getPathExpansionLimit() {
+        return getIntOrDefault(ENV_PATH_EXPANSION_LIMIT, DEFAULT_PATH_EXPANSION_LIMIT);
+    }
+
     private static int getIntOrDefault(String name, int defaultValue) {
         String strVal = System.getProperty(name);
         return strVal == null ? defaultValue : Integer.parseInt(strVal);

sfge/src/main/java/com/salesforce/config/EnvUtil.java

@@ -25,4 +25,10 @@ public interface SfgeConfig {
      * provided
      */
     int getProgressIncrements();
+
+    /** Stack depth upto which Graph Engine attempts to walk. */
+    int getStackDepthLimit();
+
+    /** Limit to control the growth of path expansion to help alleviate OutOfMemoryError. */
+    int getPathExpansionLimit();
 }

sfge/src/main/java/com/salesforce/config/SfgeConfig.java

@@ -30,6 +30,16 @@ public int getProgressIncrements() {
         return EnvUtil.getProgressIncrements();
     }
 
+    @Override
+    public int getStackDepthLimit() {
+        return EnvUtil.getStackDepthLimit();
+    }
+
+    @Override
+    public int getPathExpansionLimit() {
+        return EnvUtil.getPathExpansionLimit();
+    }
+
     static SfgeConfigImpl getInstance() {
         return SfgeConfigImpl.LazyHolder.INSTANCE;
     }

sfge/src/main/java/com/salesforce/config/SfgeConfigImpl.java

@@ -20,6 +20,8 @@ public static final class RuleViolationTemplates {
         // Format: First %s is either "abstract class" or "interface".
         //         Second %s is the name of a class or interface.
         public static final String UNIMPLEMENTED_TYPE_RULE = "Extend, implement, or delete %s %s";
+        public static final String LIMIT_REACHED_VIOLATION_MESSAGE =
+                "%s. The analysis preemptively stopped running on this path to prevent an OutOfMemory error. Rerun Graph Engine targeting this entry method with a larger heap space.";
     }
 
     /** Main args and process checks * */
@@ -37,7 +39,10 @@ public static final class RuleViolationTemplates {
             "Remove unreachable code to proceed with the analysis: %s,%s:%d";
 
     public static final String VARIABLE_DECLARED_MULTIPLE_TIMES =
-            "Rename or remove reused variable to proceed with analysis: %s,%s:%d";
+            "This variable is reused. Rename or delete it to proceed with the analysis: %s,%s:%d";
+
+    public static final String INSUFFICIENT_HEAP_SPACE =
+            "There's insufficient heap space (%d bytes) to execute Graph Engine. Increase heap space using --sfgejvmargs option and retry.";
 
     public static final String STRIP_INACCESSIBLE_READ_WARNING_TEMPLATE =
             "For stripInaccessible checks on READ operation, Salesforce Graph Engine can't verify that only sanitized data is used after the check. Discard unsanitized data for [%2$s].";
@@ -51,7 +56,11 @@ public static final class RuleViolationTemplates {
 
     public static final String INVALID_SYNTAX_TEMPLATE = "Invalid syntax at %d:%d. (%s)";
 
-    public static final String FIX_COMPILATION_ERRORS = "Fix compilation errors in %s and retry";
+    public static final String FIX_COMPILATION_ERRORS =
+            "Graph engine encountered compilation errors. Fix the errors in %s and retry.";
 
     public static final String EXCEPTION_FORMAT_TEMPLATE = "%s, Caused by:\n%s";
+
+    public static final String PATH_EXPANSION_LIMIT_REACHED =
+            "Graph Engine reached the path expansion upper limit (%d).";
 }

sfge/src/main/java/com/salesforce/config/UserFacingMessages.java

@@ -39,7 +39,7 @@
  */
 @NotThreadSafe
 @SuppressWarnings("PMD.NullAssignment")
-public final class ApexPath implements DeepCloneable<ApexPath>, Collectible<ApexPath> {
+public class ApexPath implements DeepCloneable<ApexPath>, Collectible<ApexPath> {
     public static final NullCollectible<ApexPath> NULL_VALUE =
             new NullCollectible<>(ApexPath.class);
 

sfge/src/main/java/com/salesforce/graph/ApexPath.java

@@ -2,6 +2,7 @@
 
 import com.salesforce.Collectible;
 import com.salesforce.collections.CollectionUtil;
+import com.salesforce.config.SfgeConfigProvider;
 import com.salesforce.exception.ProgrammingException;
 import com.salesforce.exception.SfgeInterruptedException;
 import com.salesforce.exception.UnexpectedException;
@@ -71,10 +72,13 @@
  * into {@link ApexPathCollapser} in order to provide information which may also result in the
  * current path getting collapsed.
  */
-final class ApexPathExpander
+class ApexPathExpander
         implements ClassStaticScopeProvider, EngineDirectiveContextProvider, Registrable {
     private static final Logger LOGGER = LogManager.getLogger(ApexPathExpander.class);
 
+    /** Allowed stack depth limit */
+    public final int stackDepthLimit;
+
     /** Used to give each object a unique id */
     private static final AtomicLong ID_GENERATOR = new AtomicLong();
 
@@ -92,6 +96,8 @@ final class ApexPathExpander
     /** Dynamically generated id used to establish object equality */
     private final Long id;
 
+    private int stackDepth;
+
     /**
      * The vertex that caused a {@link MethodPathForkedException} to be thrown and the ForkEvent
      * that was included in the exception. This ForkEvent creates a common ancestry for all
@@ -195,6 +201,8 @@ final class ApexPathExpander
         this.topMostPath = new Stack<>();
         this.topMostPath.push(topMostPath);
         this.config = config;
+        this.stackDepthLimit = SfgeConfigProvider.get().getStackDepthLimit();
+        this.stackDepth = 0;
         this.symbolProviderVisitor =
                 new DefaultSymbolProviderVertexVisitor(
                         g, config.instantiateClassScope(g).orElse(null));
@@ -242,6 +250,10 @@ final class ApexPathExpander
         this.topMostPath = CloneUtil.cloneStack(other.topMostPath);
         this.config = CloneUtil.cloneImmutable(other.config);
 
+        this.stackDepthLimit = SfgeConfigProvider.get().getStackDepthLimit();
+        // Reset stack depth
+        this.stackDepth = 0;
+
         // Find the method call that caused the fork and hook up the path
         BaseSFVertex topLevelVertex = ex.getTopLevelVertex();
         InvocableVertex invocable = ex.getInvocable();
@@ -641,12 +653,16 @@ public void visit(AbstractPathBasedRule rule) {
                 }
             }
 
+            // Increment stack depth before visiting a method. Decrement stack depth when returning.
+            incrementStackDepth(vertex);
             visit(methodPath);
+            decrementStackDepth(vertex);
 
             if (afterMethodCalled.add(pathInvocableCall)) {
                 Optional<ApexValue<?>> lastReturnValue =
                         symbolProviderVisitor.afterMethodCall(
                                 invocable, methodPath.getMethodVertex().get());
+
                 if (LOGGER.isTraceEnabled()) {
                     LOGGER.trace(
                             "lastReturnValue="
@@ -803,6 +819,26 @@ private Optional<ApexPath> resolveMethodCall(ApexPath path, InvocableVertex invo
         return result;
     }
 
+    private void incrementStackDepth(BaseSFVertex vertex) {
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Incrementing stack depth (" + stackDepth + ") for vertex: " + vertex);
+        }
+        stackDepth++;
+        if (stackDepth > stackDepthLimit) {
+            throw new StackDepthLimitExceededException(stackDepth, vertex);
+        }
+    }
+
+    private void decrementStackDepth(BaseSFVertex vertex) {
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Decrementing stack depth (" + stackDepth + ")");
+        }
+        stackDepth--;
+        if (stackDepth < 0) {
+            throw new UnexpectedException("Stack depth cannot be less than 0. vertex=" + vertex);
+        }
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;