SONARPY-489 Rule S2092: Creating cookies without the "secure" flag is security-sensitive

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

@@ -39,6 +39,7 @@
 import org.sonar.python.checks.hotspots.PubliclyWritableDirectoriesCheck;
 import org.sonar.python.checks.hotspots.RegexCheck;
 import org.sonar.python.checks.hotspots.SQLQueriesCheck;
+import org.sonar.python.checks.hotspots.SecureCookieCheck;
 import org.sonar.python.checks.hotspots.StandardInputCheck;
 import org.sonar.python.checks.hotspots.StrongCryptographicKeysCheck;
 
@@ -127,6 +128,7 @@ public static Iterable<Class> getChecks() {
       ReturnYieldOutsideFunctionCheck.class,
       SameBranchCheck.class,
       SameConditionCheck.class,
+      SecureCookieCheck.class,
       SelfAssignmentCheck.class,
       SQLQueriesCheck.class,
       StandardInputCheck.class,

python-checks/src/main/java/org/sonar/python/checks/hotspots/SecureCookieCheck.java

@@ -0,0 +1,186 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2020 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.hotspots;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Stream;
+import javax.annotation.CheckForNull;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.symbols.Symbol;
+import org.sonar.plugins.python.api.tree.Argument;
+import org.sonar.plugins.python.api.tree.AssignmentStatement;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.ExpressionList;
+import org.sonar.plugins.python.api.tree.HasSymbol;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.StringLiteral;
+import org.sonar.plugins.python.api.tree.SubscriptionExpression;
+import org.sonar.plugins.python.api.tree.Tree.Kind;
+import org.sonar.python.semantic.SymbolUtils;
+
+import static org.sonar.python.checks.Expressions.isFalsy;
+
+@Rule(key = "S2092")
+public class SecureCookieCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Make sure creating this cookie without the \"secure\" flag is safe.";
+  private static Map<String, Integer> sensitiveArgumentByFQN;
+  static {
+    sensitiveArgumentByFQN = new HashMap<>();
+    sensitiveArgumentByFQN.put("django.http.HttpResponse.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponse.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseRedirect.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseRedirect.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponsePermanentRedirect.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponsePermanentRedirect.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseNotModified.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseNotModified.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseBadRequest.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseBadRequest.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseNotFound.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseNotFound.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseForbidden.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseForbidden.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseNotAllowed.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseNotAllowed.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseGone.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseGone.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseServerError.set_cookie", 6);
+    sensitiveArgumentByFQN.put("django.http.HttpResponseServerError.set_signed_cookie", 7);
+    sensitiveArgumentByFQN.put("flask.Response.set_cookie", 6);
+    sensitiveArgumentByFQN = Collections.unmodifiableMap(sensitiveArgumentByFQN);
+  }
+
+  private static final String SECURE = "secure";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Kind.ASSIGNMENT_STMT, ctx -> {
+      AssignmentStatement assignment = (AssignmentStatement) ctx.syntaxNode();
+      getSubscriptionToCookies(assignment.lhsExpressions())
+        .forEach(sub -> {
+          if (isSettingSecureFlag(sub) && isFalsy(assignment.assignedValue())) {
+            ctx.addIssue(assignment, MESSAGE);
+          }
+        });
+    });
+
+    context.registerSyntaxNodeConsumer(Kind.CALL_EXPR, ctx -> {
+      CallExpression callExpression = (CallExpression) ctx.syntaxNode();
+      Symbol calleeSymbol = callExpression.calleeSymbol();
+      if (calleeSymbol != null && sensitiveArgumentByFQN.containsKey(calleeSymbol.fullyQualifiedName())) {
+        if (callExpression.arguments().stream().anyMatch(argument -> argument.is(Kind.UNPACKING_EXPR))) {
+          return;
+        }
+        RegularArgument secureArgument = getSecureArgument(callExpression.arguments(), sensitiveArgumentByFQN.get(calleeSymbol.fullyQualifiedName()));
+        if (secureArgument == null || isFalsy(secureArgument.expression())) {
+          ctx.addIssue(callExpression.callee(), MESSAGE);
+        }
+      }
+    });
+  }
+
+  @CheckForNull
+  private static RegularArgument getSecureArgument(List<Argument> arguments, int nArg) {
+    return arguments.stream()
+      // in call site of this function, argument will always be of type RegularArgument because we check no argument is unpacking
+      .map(RegularArgument.class::cast)
+      .filter(argument -> {
+        Name keywordArgument = argument.keywordArgument();
+        return keywordArgument != null && keywordArgument.name().equals(SECURE);
+      })
+      .findFirst().orElseGet(() -> checkSensitivePositionalArgument(arguments, nArg));
+  }
+
+  private static RegularArgument checkSensitivePositionalArgument(List<Argument> arguments, int nArg) {
+    for (int i = 0; i < arguments.size(); i++) {
+      // in call site of this function, argument will always be of type RegularArgument because we check no argument is unpacking
+      RegularArgument arg = (RegularArgument) arguments.get(i);
+      if (i == nArg) {
+        return arg;
+      }
+      if (arg.keywordArgument() != null) {
+        // not positional anymore
+        return null;
+      }
+    }
+    return null;
+  }
+
+  private static Stream<SubscriptionExpression> getSubscriptionToCookies(List<ExpressionList> lhsExpressions) {
+    return lhsExpressions.stream()
+      .flatMap(expressionList -> expressionList.expressions().stream())
+      .filter(lhs -> {
+        if (lhs.is(Kind.SUBSCRIPTION)) {
+          SubscriptionExpression sub = (SubscriptionExpression) lhs;
+          Symbol objectSymbol = getObjectSymbol(sub.object());
+          return "http.cookies.SimpleCookie".equals(SymbolUtils.getTypeName(objectSymbol));
+        }
+        return false;
+      })
+      .map(SubscriptionExpression.class::cast);
+  }
+
+  private static boolean isSettingSecureFlag(SubscriptionExpression sub) {
+    List<ExpressionList> subscripts = getSubscripts(sub);
+    if (subscripts.size() == 1) {
+      return false;
+    }
+    return subscripts.stream()
+      .skip(1)
+      .anyMatch(s -> s.expressions().size() == 1 && isSecureStringLiteral(s.expressions().get(0)));
+  }
+
+  private static List<ExpressionList> getSubscripts(SubscriptionExpression sub) {
+    Deque<ExpressionList> subscripts = new ArrayDeque<>();
+    subscripts.addFirst(sub.subscripts());
+    Expression object = sub.object();
+    while (object.is(Kind.SUBSCRIPTION)) {
+      subscripts.addFirst(((SubscriptionExpression) object).subscripts());
+      object = ((SubscriptionExpression) object).object();
+    }
+    return new ArrayList<>(subscripts);
+  }
+
+  private static boolean isSecureStringLiteral(Expression expression) {
+    return expression.is(Kind.STRING_LITERAL) && ((StringLiteral) expression).trimmedQuotesValue().equalsIgnoreCase(SECURE);
+  }
+
+  @CheckForNull
+  private static Symbol getObjectSymbol(Expression object) {
+    if (object.is(Kind.SUBSCRIPTION)) {
+      return getObjectSymbol(((SubscriptionExpression) object).object());
+    }
+    if (object instanceof HasSymbol) {
+      return ((HasSymbol) object).symbol();
+    }
+    return null;
+  }
+
+}

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

@@ -0,0 +1,63 @@
+<p>When a cookie is protected with the <code>secure</code> attribute set to <em>true</em> it will not be send by the browser over an unencrypted HTTP
+request and thus cannot be observed by an unauthorized person during a man-in-the-middle attack. </p>
+<h2>Ask Yourself Whether</h2>
+<ul>
+  <li> the cookie is for instance a <em>session-cookie</em> not designed to be sent over non-HTTPS communication. </li>
+  <li> it's not sure that the website contains <a href="https://developer.mozilla.org/fr/docs/S%C3%A9curit%C3%A9/MixedContent">mixed content</a> or
+  not (ie HTTPS everywhere or not) </li>
+</ul>
+<p>You are at risk if you answered yes to any of those questions.</p>
+<h2>Recommended Secure Coding Practices</h2>
+<ul>
+  <li> It is recommended to use <code>HTTPs</code> everywhere so setting the <code>secure</code> flag to <em>true</em> should be the default behaviour
+  when creating cookies. </li>
+  <li> Set the <code>secure</code> flag to <em>true</em> for session-cookies. </li>
+</ul>
+<h2>Sensitive Code Examples</h2>
+<p>http.cookies</p>
+<pre>
+import http.cookies
+
+cookie = http.cookies.SimpleCookie()
+cookie['key'] = 'value'
+cookie['key']['secure'] = False # Sensitive
+</pre>
+<p>Flask</p>
+<pre>
+from flask import Response
+
+@app.route('/')
+def index():
+    response = Response()
+    response.set_cookie('key', 'value') # Sensitive
+    return response
+</pre>
+<h2>Compliant Solution</h2>
+<p>http.cookies</p>
+<pre>
+import http.cookies
+
+cookie = http.cookies.SimpleCookie()
+cookie['key'] = 'value'
+cookie['key']['secure'] = True # Compliant
+</pre>
+<p>Flask</p>
+<pre>
+from flask import Response
+
+@app.route('/')
+def index():
+    response = Response()
+    response.set_cookie('key', 'value', secure=True) # Compliant
+    return response
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://www.owasp.org/index.php/Top_10-2017_A3-Sensitive_Data_Exposure">OWASP Top 10 2017 Category A3</a> - Sensitive Data Exposure
+  </li>
+  <li> <a href="http://cwe.mitre.org/data/definitions/311">MITRE, CWE-311</a> - Missing Encryption of Sensitive Data </li>
+  <li> <a href="http://cwe.mitre.org/data/definitions/315">MITRE, CWE-315</a> - Cleartext Storage of Sensitive Information in a Cookie </li>
+  <li> <a href="http://cwe.mitre.org/data/definitions/614">MITRE, CWE-614</a> - Sensitive Cookie in HTTPS Session Without 'Secure' Attribute </li>
+  <li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
+</ul>
+

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

@@ -0,0 +1,29 @@
+{
+  "title": "Creating cookies without the \"secure\" flag is security-sensitive",
+  "type": "SECURITY_HOTSPOT",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "cwe",
+    "privacy",
+    "sans-top25-porous",
+    "owasp-a3"
+  ],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-2092",
+  "sqKey": "S2092",
+  "scope": "Main",
+  "securityStandards": {
+    "CWE": [
+      614,
+      311,
+      315
+    ],
+    "OWASP": [
+      "A3"
+    ]
+  }
+}

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

@@ -37,6 +37,7 @@
     "S2068",
     "S2053",
     "S2077",
+    "S2092",
     "S2190",
     "S2245",
     "S2711",

python-checks/src/test/java/org/sonar/python/checks/hotspots/SecureCookieCheckTest.java

@@ -0,0 +1,31 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2020 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.hotspots;
+
+import java.util.Collections;
+import org.junit.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+public class SecureCookieCheckTest {
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify(Collections.singletonList("src/test/resources/checks/hotspots/secureCookie.py"), new SecureCookieCheck());
+  }
+}