Merge pull request #807 from markshannon/python-insecure-file-permission

Python: Weak file permissions query.

change-notes/1.20/analysis-python.md

@@ -14,6 +14,7 @@ Removes false positives seen when using Python 3.6, but not when using earlier v
  | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
 | Default version of SSL/TLS may be insecure (`py/insecure-default-protocol`) | security, external/cwe/cwe-327 | Finds instances where an insecure default protocol may be used. Results are shown on LGTM by default. |
+| Overly permissive file permissions (`py/overly-permissive-file`) | security, external/cwe/cwe-732 | Finds instances where a file is created with overly permissive permissions. Results are not shown on LGTM by default. |
 | Use of insecure SSL/TLS version (`py/insecure-protocol`) | security, external/cwe/cwe-327 | Finds instances where a known insecure protocol has been specified. Results are shown on LGTM by default. |
 
  ## Changes to existing queries

python/ql/src/Security/CWE-732/WeakFilePermissions.qhelp

@@ -0,0 +1,26 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+When creating a file POSIX systems allow permissions to be specified
+for owner, group and others separately. Permissions should be kept as
+strict as possible, preventing access to the files contents by other users.
+</p>
+
+</overview>
+
+<recommendation>
+<p>
+Restrict the file permissions of files to prevent any but the owner being able to read or write to that file
+</p>
+</recommendation>
+
+<references>
+<li>
+Wikipedia:
+<a href="https://en.wikipedia.org/wiki/File_system_permissions">File system permissions</a>.
+</li>
+</references>
+
+</qhelp>

python/ql/src/Security/CWE-732/WeakFilePermissions.ql

@@ -0,0 +1,53 @@
+/**
+ * @name Overly permissive file permissions
+ * @description Allowing files to be readable or writable by users other than the owner may allow sensitive information to be accessed.
+ * @kind problem
+ * @id py/overly-permissive-file
+ * @problem.severity warning
+ * @sub-severity high
+ * @precision medium
+ * @tags external/cwe/cwe-732
+ *       security
+ */
+import python
+
+bindingset[p]
+int world_permission(int p) {
+    result = p % 8
+}
+
+bindingset[p]
+int group_permission(int p) {
+    result = (p/8) % 8
+}
+
+bindingset[p]
+string access(int p) {
+    p%4 >= 2 and result = "writable" or
+    p%4 < 2 and p != 0 and result = "readable"
+}
+
+bindingset[p]
+string permissive_permission(int p) {
+    result = "world " + access(world_permission(p))
+    or
+    world_permission(p) = 0 and result = "group " + access(group_permission(p))
+}
+
+predicate chmod_call(CallNode call, FunctionObject chmod, NumericObject num) {
+    any(ModuleObject os | os.getName() = "os").getAttribute("chmod") = chmod and
+    chmod.getACall() = call and call.getArg(1).refersTo(num)
+}
+
+predicate open_call(CallNode call, FunctionObject open, NumericObject num) {
+    any(ModuleObject os | os.getName() = "os").getAttribute("open") = open and
+    open.getACall() = call and call.getArg(2).refersTo(num)
+}
+
+
+from CallNode call, FunctionObject func, NumericObject num, string permission
+where
+    (chmod_call(call, func, num) or open_call(call, func, num))
+    and
+    permission = permissive_permission(num.intValue())
+select call, "Overly permissive mask in " + func.getName() + " sets file to " + permission + "."

python/ql/test/query-tests/Security/CWE-732/WeakFilePermissions.expected

@@ -0,0 +1,7 @@
+| test.py:7:1:7:19 | ControlFlowNode for Attribute() | Overly permissive mask in chmod sets file to world writable. |
+| test.py:8:1:8:20 | ControlFlowNode for Attribute() | Overly permissive mask in chmod sets file to world writable. |
+| test.py:9:1:9:21 | ControlFlowNode for Attribute() | Overly permissive mask in chmod sets file to world writable. |
+| test.py:11:1:11:21 | ControlFlowNode for Attribute() | Overly permissive mask in chmod sets file to group readable. |
+| test.py:13:1:13:28 | ControlFlowNode for Attribute() | Overly permissive mask in chmod sets file to group writable. |
+| test.py:14:1:14:19 | ControlFlowNode for Attribute() | Overly permissive mask in chmod sets file to group writable. |
+| test.py:16:1:16:25 | ControlFlowNode for Attribute() | Overly permissive mask in open sets file to world readable. |

python/ql/test/query-tests/Security/CWE-732/WeakFilePermissions.qlref

@@ -0,0 +1 @@
+Security/CWE-732/WeakFilePermissions.ql

python/ql/test/query-tests/Security/CWE-732/options

@@ -0,0 +1,2 @@
+semmle-extractor-options: --max-import-depth=2 -p ../lib
+optimize: true

python/ql/test/query-tests/Security/CWE-732/test.py

@@ -0,0 +1,16 @@
+import os
+import stat
+
+file = 'semmle/important_secrets'
+
+
+os.chmod(file, 0o7) # BAD
+os.chmod(file, 0o77) # BAD
+os.chmod(file, 0o777) # BAD
+os.chmod(file, 0o600) # GOOD
+os.chmod(file, 0o550) # BAD
+os.chmod(file, stat.S_IRWXU) # GOOD
+os.chmod(file, stat.S_IWGRP) # BAD
+os.chmod(file, 400) # BAD -- Decimal format.
+
+os.open(file, 'w', 0o704) # BAD

python/ql/test/query-tests/Security/lib/os/__init__.py

@@ -3,3 +3,9 @@ def system(cmd, *args, **kwargs):
 
 def popen(cmd, *args, **kwargs):
     return None
+
+def chmod(path, mode):
+    pass
+
+def open(path, flags, mode):
+    pass