SONARPY-1013 Rule S6252: Disabling versioning of S3 buckets is security-sensitive (#1123)

Author: marco-bearzi-sonarsource

python-checks/src/main/java/org/sonar/python/checks/CheckList.java

@@ -22,6 +22,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import org.sonar.python.checks.cdk.S3BucketVersioningCheck;
 import org.sonar.python.checks.hotspots.ClearTextProtocolsCheck;
 import org.sonar.python.checks.hotspots.CommandLineArgsCheck;
 import org.sonar.python.checks.hotspots.CorsCheck;
@@ -51,8 +52,8 @@
 import org.sonar.python.checks.hotspots.UnverifiedHostnameCheck;
 import org.sonar.python.checks.regex.AnchorPrecedenceCheck;
 import org.sonar.python.checks.regex.DuplicatesInCharacterClassCheck;
-import org.sonar.python.checks.regex.EmptyGroupCheck;
 import org.sonar.python.checks.regex.EmptyAlternativeCheck;
+import org.sonar.python.checks.regex.EmptyGroupCheck;
 import org.sonar.python.checks.regex.EmptyStringRepetitionCheck;
 import org.sonar.python.checks.regex.GraphemeClustersInClassesCheck;
 import org.sonar.python.checks.regex.GroupReplacementCheck;
@@ -67,9 +68,9 @@
 import org.sonar.python.checks.regex.SingleCharCharacterClassCheck;
 import org.sonar.python.checks.regex.SingleCharacterAlternationCheck;
 import org.sonar.python.checks.regex.StringReplaceCheck;
+import org.sonar.python.checks.regex.SuperfluousCurlyBraceCheck;
 import org.sonar.python.checks.regex.UnquantifiedNonCapturingGroupCheck;
 import org.sonar.python.checks.regex.VerboseRegexCheck;
-import org.sonar.python.checks.regex.SuperfluousCurlyBraceCheck;
 
 public final class CheckList {
 
@@ -224,6 +225,7 @@ public static Iterable<Class> getChecks() {
       ReturnAndYieldInOneFunctionCheck.class,
       ReturnYieldOutsideFunctionCheck.class,
       RobustCipherAlgorithmCheck.class,
+      S3BucketVersioningCheck.class,
       SameBranchCheck.class,
       SameConditionCheck.class,
       SecureCookieCheck.class,

python-checks/src/main/java/org/sonar/python/checks/cdk/S3BucketVersioningCheck.java

@@ -0,0 +1,98 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks.cdk;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.tree.Argument;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.Token;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.python.checks.Expressions;
+
+@Rule(key = "S6252")
+public class S3BucketVersioningCheck extends PythonSubscriptionCheck {
+
+  public static final String MESSAGE = "Make sure an unversioned S3 bucket is safe here.";
+  public static final String MESSAGE_OMITTING = "Omitting the \"versioned\" argument disables S3 bucket versioning. Make sure it is safe here.";
+  public static final String MESSAGE_SECONDARY = "Propagated setting.";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::visitNode);
+  }
+
+  public void visitNode(SubscriptionContext ctx) {
+    CallExpression node = (CallExpression) ctx.syntaxNode();
+    Optional.ofNullable(node.calleeSymbol())
+      .filter(nodeSymbol -> ("aws_cdk.aws_s3.Bucket".equals(nodeSymbol.fullyQualifiedName())))
+      .ifPresent(nodeSymbol -> {
+        Optional<RegularArgument> version = getVersionArgument(node.arguments());
+        if (version.isPresent()) {
+          version.map(regArg -> propagatedSensitiveExpressions(regArg.expression()))
+            .filter(trees -> !trees.isEmpty())
+            .ifPresent(trees -> {
+              PreciseIssue issue = ctx.addIssue(trees.get(0).parent(), MESSAGE);
+              trees.stream().skip(1).forEach(expression -> issue.secondary(expression.parent(), MESSAGE_SECONDARY));
+            });
+        } else {
+          ctx.addIssue(node.callee(), MESSAGE_OMITTING);
+        }
+      });
+  }
+
+  private static Optional<RegularArgument> getVersionArgument(List<Argument> args) {
+    return args.stream()
+      .map(RegularArgument.class::cast)
+      .filter(a -> a.keywordArgument() != null)
+      .filter(a -> "versioned".equals(a.keywordArgument().name()))
+      .findAny();
+  }
+
+  private static List<Tree> propagatedSensitiveExpressions(Expression expression) {
+    List<Tree> trees = new ArrayList<>();
+    if (Optional.ofNullable(expression.firstToken()).filter(S3BucketVersioningCheck::isFalse).isPresent()) {
+      trees.add(expression);
+    } else if (expression.is(Tree.Kind.NAME)) {
+      Expression singleAssignedValue = Expressions.singleAssignedValue(((Name) expression));
+      if (singleAssignedValue != null) {
+        trees.add(expression);
+        List<Tree> subTrees = propagatedSensitiveExpressions(singleAssignedValue);
+        if (subTrees.isEmpty()) {
+          return Collections.emptyList();
+        }
+        trees.addAll(subTrees);
+      }
+    }
+    return trees;
+  }
+
+  private static boolean isFalse(Token token) {
+    return "False".equals(token.value());
+  }
+}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6252.html

@@ -0,0 +1,33 @@
+<p>S3 buckets can be versioned. When the S3 bucket is unversioned it means that a new version of an object overwrites an existing one in the S3
+bucket.</p>
+<p>It can lead to unintentional or intentional information loss.</p>
+<h2>Ask Yourself Whether</h2>
+<ul>
+  <li> The bucket stores information that require high availability. </li>
+</ul>
+<p>There is a risk if you answered yes to any of those questions.</p>
+<h2>Recommended Secure Coding Practices</h2>
+<p>It’s recommended to enable S3 versioning and thus to have the possibility to retrieve and restore different versions of an object.</p>
+<h2>Sensitive Code Example</h2>
+<pre>
+bucket = s3.Bucket(self, "MyUnversionedBucket",
+    versioned=False       # Sensitive
+)
+</pre>
+<p>The default value of <code>versioned</code> is <code>False</code> so the absence of this parameter is also sensitive.</p>
+<h2>Compliant Solution</h2>
+<pre>
+bucket = s3.Bucket(self, "MyVersionedBucket",
+    versioned=True       # Compliant
+)
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://owasp.org/Top10/A05_2021-Security_Misconfiguration/">OWASP Top 10 2021 Category A5</a> - Security Misconfiguration </li>
+  <li> <a href="https://www.owasp.org/index.php/Top_10-2017_A6-Security_Misconfiguration">OWASP Top 10 2017 Category A6</a> - Security
+  Misconfiguration </li>
+  <li> <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/Versioning.html">AWS documentation</a> - Using versioning in S3 buckets </li>
+  <li> <a href="https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3.Bucket.html#versioned">CDK documentation</a> - Using versioning in S3
+  buckets </li>
+</ul>
+

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S6252.json

@@ -0,0 +1,25 @@
+{
+  "title": "Disabling versioning of S3 buckets is security-sensitive",
+  "type": "SECURITY_HOTSPOT",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "aws",
+    "owasp-a6"
+  ],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-6252",
+  "sqKey": "S6252",
+  "scope": "Main",
+  "securityStandards": {
+    "OWASP": [
+      "A6"
+    ],
+    "OWASP Top 10 2021": [
+      "A5"
+    ]
+  }
+}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json

@@ -152,6 +152,7 @@
     "S6002",
     "S6019",
     "S6035",
+    "S6252",
     "S6323",
     "S6326",
     "S6328",

python-checks/src/test/java/org/sonar/python/checks/cdk/S3BucketVersioningCheckTest.java

@@ -0,0 +1,32 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.python.checks.cdk;
+
+import org.junit.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+public class S3BucketVersioningCheckTest {
+
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/cdk/s3BucketVersioning.py", new S3BucketVersioningCheck());
+  }
+
+}

python-checks/src/test/resources/checks/cdk/s3BucketVersioning.py

@@ -0,0 +1,33 @@
+from aws_cdk import aws_s3 as s3
+
+# Noncompliant@+1 {{Omitting the "versioned" argument disables S3 bucket versioning. Make sure it is safe here.}}
+bucket = s3.Bucket(self, "MyUnversionedBucket")
+#        ^^^^^^^^^
+
+bucket = s3.Bucket(self, "MyUnversionedBucket", versioned=False)  # Noncompliant {{Make sure an unversioned S3 bucket is safe here.}}
+#                                               ^^^^^^^^^^^^^^^
+
+bucket = s3.Bucket(self, "MyUnversionedBucket", versioned=True)  # Compliant
+
+bucket = s3.Bucket(self, "MyUnversionedBucket", versioned=unresolved_var)  # Compliant
+
+
+versioning = True
+bucket = s3.Bucket(self, "MyUnversionedBucket", versioned=versioning)  # Compliant
+
+value = "FALSE"
+bucket = s3.Bucket(self, "MyUnversionedBucket", versioned=value)  # Compliant
+
+
+a = function_call_for_coverage(10)
+
+
+def test():
+    bucket_versioning = False
+#   ^^^^^^^^^^^^^^^^^^^^^^^^^> {{Propagated setting.}}
+    second_versioning = bucket_versioning
+#   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^> {{Propagated setting.}}
+    bucket = s3.Bucket(self, "MyUnversionedBucket",
+                       versioned=second_versioning  # Noncompliant {{Make sure an unversioned S3 bucket is safe here.}}
+                    #  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+                       )