SONARPY-376 Rule S5443: Using publicly writable directories is securi…

its/ruling/src/test/resources/expected/python-S5443.json

@@ -0,0 +1,57 @@
+{
+'project:buildbot-0.8.6p1/contrib/bb_applet.py':[
+30,
+174,
+],
+'project:docker-compose-1.24.1/tests/acceptance/cli_test.py':[
+2617,
+],
+'project:docker-compose-1.24.1/tests/integration/service_test.py':[
+284,
+303,
+448,
+706,
+],
+'project:docker-compose-1.24.1/tests/unit/config/config_test.py':[
+1352,
+1367,
+4306,
+],
+'project:tornado-2.3/demos/s3server/s3server.py':[
+47,
+],
+'project:twisted-12.1.0/doc/core/examples/courier.py':[
+10,
+],
+'project:twisted-12.1.0/doc/core/examples/ptyserv.py':[
+18,
+],
+'project:twisted-12.1.0/doc/core/examples/rotatinglog.py':[
+14,
+],
+'project:twisted-12.1.0/twisted/names/authority.py':[
+18,
+],
+'project:twisted-12.1.0/twisted/test/test_amp.py':[
+2989,
+2993,
+],
+'project:twisted-12.1.0/twisted/test/test_ftp.py':[
+1870,
+1885,
+1901,
+1916,
+1930,
+1945,
+1961,
+1976,
+],
+'project:twisted-12.1.0/twisted/test/test_process.py':[
+2000,
+2013,
+],
+'project:twisted-12.1.0/twisted/web/test/test_template.py':[
+185,
+193,
+],
+}

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

@@ -29,6 +29,7 @@
 import org.sonar.python.checks.hotspots.HashingDataCheck;
 import org.sonar.python.checks.hotspots.OsExecCheck;
 import org.sonar.python.checks.hotspots.ProcessSignallingCheck;
+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.StandardInputCheck;
@@ -90,6 +91,7 @@ public static Iterable<Class> getChecks() {
       PreIncrementDecrementCheck.class,
       PrintStatementUsageCheck.class,
       ProcessSignallingCheck.class,
+      PubliclyWritableDirectoriesCheck.class,
       RegexCheck.class,
       ReturnAndYieldInOneFunctionCheck.class,
       ReturnYieldOutsideFunctionCheck.class,

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

@@ -0,0 +1,116 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2019 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.Arrays;
+import java.util.List;
+import java.util.Locale;
+import java.util.regex.Pattern;
+import org.sonar.check.Rule;
+import org.sonar.python.PythonSubscriptionCheck;
+import org.sonar.python.api.tree.PyArgumentTree;
+import org.sonar.python.api.tree.PyCallExpressionTree;
+import org.sonar.python.api.tree.PyExpressionTree;
+import org.sonar.python.api.tree.PyNameTree;
+import org.sonar.python.api.tree.PyQualifiedExpressionTree;
+import org.sonar.python.api.tree.PyStringLiteralTree;
+import org.sonar.python.api.tree.PySubscriptionExpressionTree;
+import org.sonar.python.api.tree.Tree.Kind;
+import org.sonar.python.semantic.Symbol;
+import org.sonar.python.semantic.SymbolTable;
+
+@Rule(key = "S5443")
+public class PubliclyWritableDirectoriesCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Make sure publicly writable directories are used safely here.";
+  private static final List<String> UNIX_WRITABLE_DIRECTORIES = Arrays.asList(
+    "/tmp/", "/var/tmp/", "/usr/tmp/", "/dev/shm/", "/dev/mqueue/", "/run/lock/", "/var/run/lock/",
+    "/library/caches/", "/users/shared/", "/private/tmp/", "/private/var/tmp/");
+  private static final List<String> NONCOMPLIANT_ENVIRON_VARIABLES = Arrays.asList("tmpdir", "tmp");
+
+  private static final Pattern WINDOWS_WRITABLE_DIRECTORIES = Pattern.compile("[^\\\\]*\\\\(Windows\\\\Temp|Temp|TMP)(\\\\.*|$)", Pattern.CASE_INSENSITIVE);
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Kind.STRING_LITERAL, ctx -> {
+      PyStringLiteralTree tree = (PyStringLiteralTree) ctx.syntaxNode();
+      String stringLiteral = tree.trimmedQuotesValue().toLowerCase(Locale.ENGLISH);
+      if (UNIX_WRITABLE_DIRECTORIES.stream().anyMatch(dir -> containsDirectory(stringLiteral, dir)) ||
+        WINDOWS_WRITABLE_DIRECTORIES.matcher(stringLiteral).matches()) {
+        ctx.addIssue(tree, MESSAGE);
+      }
+    });
+
+    context.registerSyntaxNodeConsumer(Kind.CALL_EXPR, ctx -> {
+      PyCallExpressionTree tree = (PyCallExpressionTree) ctx.syntaxNode();
+      if (isOsEnvironGetter(tree.callee(), ctx.symbolTable()) &&
+        tree.arguments().stream().map(PyArgumentTree::expression)
+          .anyMatch(PubliclyWritableDirectoriesCheck::isNonCompliantOsEnvironArgument)) {
+        ctx.addIssue(tree, MESSAGE);
+      }
+    });
+
+    context.registerSyntaxNodeConsumer(Kind.SUBSCRIPTION, ctx -> {
+      PySubscriptionExpressionTree tree = (PySubscriptionExpressionTree) ctx.syntaxNode();
+      if (isOsEnvironQualifiedExpression(tree.object(), ctx.symbolTable()) && tree.subscripts().expressions().stream()
+          .anyMatch(PubliclyWritableDirectoriesCheck::isNonCompliantOsEnvironArgument)) {
+        ctx.addIssue(tree, MESSAGE);
+      }
+    });
+
+  }
+
+  private static boolean containsDirectory(String stringLiteral, String dir) {
+    return stringLiteral.startsWith(dir) || stringLiteral.equals(dir.substring(0, dir.length() - 1));
+  }
+
+  private static boolean isNonCompliantOsEnvironArgument(PyExpressionTree expression) {
+    return expression.is(Kind.STRING_LITERAL) &&
+      NONCOMPLIANT_ENVIRON_VARIABLES.contains(((PyStringLiteralTree) expression).trimmedQuotesValue().toLowerCase(Locale.ENGLISH));
+  }
+
+  // Could be either `os.environ.get` or `environ.get`
+  private static boolean isOsEnvironGetter(PyExpressionTree expression, SymbolTable symbolTable) {
+    if (!expression.is(Kind.QUALIFIED_EXPR)) {
+      return false;
+    }
+    PyQualifiedExpressionTree qualifiedExpression = (PyQualifiedExpressionTree) expression;
+    if (qualifiedExpression.name().name().equals("get")) {
+      return isOsEnvironQualifiedExpression(qualifiedExpression.qualifier(), symbolTable);
+    }
+    return false;
+  }
+
+  // Could be either `os.environ` or `from os import environ; ... ; environ`
+  private static boolean isOsEnvironQualifiedExpression(PyExpressionTree expression, SymbolTable symbolTable) {
+    Symbol symbol = symbolTable.getSymbol(expression);
+    if (symbol != null) {
+      return symbol.qualifiedName().equals("os.environ");
+    }
+    if (!expression.is(Kind.QUALIFIED_EXPR)) {
+      return false;
+    }
+    PyQualifiedExpressionTree qualifiedExpression = (PyQualifiedExpressionTree) expression;
+    if (qualifiedExpression.qualifier().is(Kind.NAME)) {
+      return ((PyNameTree) qualifiedExpression.qualifier()).name().equals("os") && qualifiedExpression.name().name().equals("environ");
+    }
+    return false;
+  }
+}

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

@@ -0,0 +1,70 @@
+<p>Operating systems have global directories where any user have write access. Those folders are mostly used as temporary storage areas like
+<code>/tmp</code> in Linux based systems. An application manipulating files from these folders is exposed to race conditions on filenames: a malicious
+user can try to create a file with a predictable name before the application does. A successful attack can result in other files being accessed,
+modified, corrupted or deleted. This risk is even higher if the application runs with elevated permissions.</p>
+<p>In the past, it has led to the following vulnerabilities:</p>
+<ul>
+  <li> <a href="https://nvd.nist.gov/vuln/detail/CVE-2012-2451">CVE-2012-2451</a> </li>
+  <li> <a href="https://nvd.nist.gov/vuln/detail/CVE-2015-1838">CVE-2015-1838</a> </li>
+</ul>
+<p>This rule raises an issue whenever it detects a hard-coded path to a publicly writable directory like <code>/tmp</code> (see complete list bellow).
+It also detects access to <code>TMP</code> and <code>TMPDIR</code> environment variables.</p>
+<ul>
+  <li> <code>/tmp</code> </li>
+  <li> <code>/var/tmp</code> </li>
+  <li> <code>/usr/tmp</code> </li>
+  <li> <code>/dev/shm</code> </li>
+  <li> <code>/dev/mqueue</code> </li>
+  <li> <code>/run/lock</code> </li>
+  <li> <code>/var/run/lock</code> </li>
+  <li> <code>/Library/Caches</code> </li>
+  <li> <code>/Users/Shared</code> </li>
+  <li> <code>/private/tmp</code> </li>
+  <li> <code>/private/var/tmp</code> </li>
+  <li> <code>\Windows\Temp</code> </li>
+  <li> <code>\Temp</code> </li>
+  <li> <code>\TMP</code> </li>
+</ul>
+<h2>Ask Yourself Whether</h2>
+<ul>
+  <li> Files are read from or written into a publicly writable folder </li>
+  <li> The application creates files with predictable names into a publicly writable folder </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> Use a dedicated sub-folder with tightly controlled permissions </li>
+  <li> Use secure-by-design APIs to create temporary files. Such API will make sure:
+    <ul>
+      <li> The generated filename is unpredictable </li>
+      <li> The file is readable and writable only by the creating user ID </li>
+      <li> The file descriptor is not inherited by child processes </li>
+      <li> The file will be destroyed as soon as it is closed </li>
+    </ul> </li>
+</ul>
+<h2>Sensitive Code Examples</h2>
+<pre>
+file = open("/tmp/temporary_file","w+") # Questionable
+</pre>
+<pre>
+tmp_dir = os.environ.get('TMPDIR') # Questionable
+file = open(tmp_dir+"/temporary_file","w+")
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+import tempfile
+
+file = tempfile.TemporaryFile(dir="/tmp/my_subdirectory", mode='"w+") # Compliant
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://www.owasp.org/index.php/Top_10-2017_A5-Broken_Access_Control">OWASP Top 10 2017 Category A5</a> - Broken Access Control </li>
+  <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/377">MITRE, CWE-377</a> - Insecure Temporary File </li>
+  <li> <a href="http://cwe.mitre.org/data/definitions/379">MITRE, CWE-379</a> - Creation of Temporary File in Directory with Incorrect Permissions
+  </li>
+  <li> <a href="https://www.owasp.org/index.php/Insecure_Temporary_File">OWASP, Insecure Temporary File</a> </li>
+  <li> <a href="https://docs.python.org/3/library/tempfile.html">Python tempfile module</a> </li>
+</ul>
+

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

@@ -0,0 +1,24 @@
+{
+  "title": "Using publicly writable directories is security-sensitive",
+  "type": "SECURITY_HOTSPOT",
+  "status": "ready",
+  "tags": [
+    "cwe",
+    "owasp-a5",
+    "owasp-a3"
+  ],
+  "defaultSeverity": "Critical",
+  "ruleSpecification": "RSPEC-5443",
+  "sqKey": "S5443",
+  "scope": "Main",
+  "securityStandards": {
+    "CWE": [
+      377,
+      379
+    ],
+    "OWASP": [
+      "A5",
+      "A3"
+    ]
+  }
+}

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

@@ -45,6 +45,7 @@
     "S4828",
     "S4829",
     "S5332",
+    "S5443",
     "S5445"
   ]
 }

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

@@ -0,0 +1,30 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2019 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 org.junit.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+public class PubliclyWritableDirectoriesCheckTest {
+  @Test
+  public void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/hotspots/publiclyWritableDirectories.py", new PubliclyWritableDirectoriesCheck());
+  }
+}

python-checks/src/test/resources/checks/hotspots/publiclyWritableDirectories.py

@@ -0,0 +1,47 @@
+# Common
+open("/tmp/f","w+") # Noncompliant
+open("/tmp","w+") # Noncompliant
+open("/var/tmp/f","w+") # Noncompliant
+open("/usr/tmp/f","w+") # Noncompliant
+open("/dev/shm/f","w+") # Noncompliant
+open("dev/shm/f","w+") # OK
+open("/tmpx","w+") # OK
+
+# Linux
+open("/dev/mqueue/f","w+") # Noncompliant
+open("/run/lock/f","w+") # Noncompliant
+open("/var/run/lock/f","w+") # Noncompliant
+
+# MacOS
+open("/Library/Caches/f","w+") # Noncompliant
+open("/Users/Shared/f","w+") # Noncompliant
+open("/private/tmp/f","w+") # Noncompliant
+open("/private/var/tmp/f","w+") # Noncompliant
+
+# Windows
+open("\Windows\Temp\f") # Noncompliant
+open("D:\Windows\Temp\f") # Noncompliant
+open("\Windows\Temp\f") # Noncompliant
+open("\Temp\f") # Noncompliant
+open("\TEMP\f") # Noncompliant
+open("\TMP\f") # Noncompliant
+open("C:\Temperatures") # OK
+
+def environ_variables():
+    import os
+    import myos
+    from os import environ
+    tmp_dir = os.environ.get('TMPDIR') # Noncompliant
+    tmp_dir = os.environ.get('TMP') # Noncompliant
+    tmp_dir = os.environ['TMPDIR'] # Noncompliant
+    tmp_dir = os.environ[foo] # OK
+    tmp_dir = os.environ.other_method('TMPDIR') # OK
+    tmp_dir = os.environ.get('OTHER') # OK
+    tmp_dir = os.environ['OTHER'] # OK
+    tmp_dir = os.environ['OTHER'] # OK
+    tmp_dir = os.other['TMPDIR'] # OK
+    tmp_dir = other['TMPDIR'] # OK
+    tmp_dir = os.foo.environ['TMPDIR'] # OK
+    tmp_dir = environ['TMPDIR'] # Noncompliant
+    tmp_dir = environ.get('TMPDIR') # Noncompliant
+    tmp_dir = myos.environ.get('TMPDIR') # OK

sonar-python-plugin/pom.xml

@@ -135,8 +135,8 @@
             <configuration>
               <rules>
                 <requireFilesSize>
-                  <maxsize>2900000</maxsize>
-                  <minsize>2600000</minsize>
+                  <maxsize>3000000</maxsize>
+                  <minsize>2700000</minsize>
                   <files>
                     <file>${project.build.directory}/${project.build.finalName}.jar</file>
                   </files>

sonar-python-plugin/src/test/java/org/sonar/plugins/python/PythonRuleRepositoryTest.java

@@ -40,7 +40,7 @@ public void createRulesTest() {
 
     List<RulesDefinition.Rule> rules = repository.rules();
     assertThat(rules).isNotNull();
-    assertThat(rules).hasSize(67);
+    assertThat(rules).hasSize(68);
 
     RulesDefinition.Rule s1578 = repository.rule("S1578");
     assertThat(s1578).isNotNull();