Merge pull request #1763 from markshannon/python-cwe-312

Python: Two new queries for CWE-312.

Author: taus-semmle

change-notes/1.23/analysis-python.md

@@ -0,0 +1,14 @@
+# Improvements to Python analysis
+
+
+## General improvements
+
+
+
+## New queries
+
+| **Query** | **Tags** | **Purpose** |
+|-----------|----------|-------------|
+| Clear-text logging of sensitive information (`py/clear-text-logging-sensitive-data`) | security, external/cwe/cwe-312 | Finds instances where sensitive information is logged without encryption or hashing. Results are shown on LGTM by default. |
+| Clear-text storage of sensitive information (`py/clear-text-storage-sensitive-data`) | security, external/cwe/cwe-312 | Finds instances where sensitive information is stored without encryption or hashing. Results are shown on LGTM by default. |
+

python/ql/src/Security/CWE-312/CleartextLogging.qhelp

@@ -0,0 +1,5 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<include src="CleartextStorage.qhelp" /></qhelp>

python/ql/src/Security/CWE-312/CleartextLogging.ql

@@ -0,0 +1,42 @@
+/**
+ * @name Clear-text logging of sensitive information
+ * @description Logging sensitive information without encryption or hashing can
+ *              expose it to an attacker.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id py/clear-text-logging-sensitive-data
+ * @tags security
+ *       external/cwe/cwe-312
+ *       external/cwe/cwe-315
+ *       external/cwe/cwe-359
+ */
+
+import python
+import semmle.python.security.Paths
+
+import semmle.python.security.TaintTracking
+import semmle.python.security.SensitiveData
+import semmle.python.security.ClearText
+
+
+class CleartextLoggingConfiguration extends TaintTracking::Configuration {
+
+    CleartextLoggingConfiguration() {  this = "ClearTextLogging" }
+
+    override predicate isSource(DataFlow::Node src, TaintKind kind) {
+        src.asCfgNode().(SensitiveData::Source).isSourceOf(kind)
+    }
+
+    override predicate isSink(DataFlow::Node sink, TaintKind kind) {
+        sink.asCfgNode() instanceof ClearTextLogging::Sink and
+        kind instanceof SensitiveData
+    }
+
+}
+
+
+from CleartextLoggingConfiguration config, TaintedPathSource source, TaintedPathSink sink
+where config.hasFlowPath(source, sink)
+select sink.getSink(), source, sink, "Sensitive data returned by $@ is logged here.",
+  source.getSource(), source.getCfgNode().(SensitiveData::Source).repr()

python/ql/src/Security/CWE-312/CleartextStorage.qhelp

@@ -0,0 +1,52 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Sensitive information that is stored unencrypted is accessible to an attacker
+who gains access to the storage. This is particularly important for cookies,
+which are stored on the machine of the end-user.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Ensure that sensitive information is always encrypted before being stored.
+If possible, avoid placing sensitive information in cookies altogether.
+Instead, prefer storing, in the cookie, a key that can be used to look up the
+sensitive information.
+</p>
+<p>
+In general, decrypt sensitive information only at the point where it is
+necessary for it to be used in cleartext.
+</p>
+
+<p>
+
+Be aware that external processes often store the <code>standard
+out</code> and <code>standard error</code> streams of the application,
+causing logged sensitive information to be stored as well.
+
+</p>
+
+</recommendation>
+
+<example>
+<p>
+The following example code stores user credentials (in this case, their password) in a cookie in plain text:
+</p>
+<sample src="examples/password_in_cookie.py"/>
+<p>
+Instead, the credentials should be encrypted, for instance by using the <code>cryptography</code> module, or not stored at all.
+</p>
+</example>
+
+<references>
+
+<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li> 
+<li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>
+
+</references>
+</qhelp>

python/ql/src/Security/CWE-312/CleartextStorage.ql

@@ -0,0 +1,41 @@
+/**
+ * @name Clear-text storage of sensitive information
+ * @description Sensitive information stored without encryption or hashing can expose it to an
+ *              attacker.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id py/clear-text-storage-sensitive-data
+ * @tags security
+ *       external/cwe/cwe-312
+ *       external/cwe/cwe-315
+ *       external/cwe/cwe-359
+ */
+
+import python
+import semmle.python.security.Paths
+
+import semmle.python.security.TaintTracking
+import semmle.python.security.SensitiveData
+import semmle.python.security.ClearText
+
+class CleartextStorageConfiguration extends TaintTracking::Configuration {
+
+    CleartextStorageConfiguration() {  this = "ClearTextStorage" }
+
+    override predicate isSource(DataFlow::Node src, TaintKind kind) {
+        src.asCfgNode().(SensitiveData::Source).isSourceOf(kind)
+    }
+
+    override predicate isSink(DataFlow::Node sink, TaintKind kind) {
+        sink.asCfgNode() instanceof ClearTextStorage::Sink and
+        kind instanceof SensitiveData
+    }
+
+}
+
+
+from CleartextStorageConfiguration config, TaintedPathSource source, TaintedPathSink sink
+where config.hasFlowPath(source, sink)
+select sink.getSink(), source, sink, "Sensitive data from $@ is stored here.",
+  source.getSource(), source.getCfgNode().(SensitiveData::Source).repr()

python/ql/src/Security/CWE-312/examples/password_in_cookie.py

@@ -0,0 +1,10 @@
+from flask import Flask, make_response, request
+
+app = Flask("Leak password")
+
+@app.route('/')
+def index():
+    password = request.args.get("password")
+    resp = make_response(render_template(...))
+    resp.set_cookie("password", password)
+    return resp

python/ql/src/semmle/python/dataflow/Files.qll

@@ -0,0 +1,26 @@
+import python
+import semmle.python.security.TaintTracking
+
+class OpenFile extends TaintKind {
+
+    OpenFile() { this = "file.open" }
+
+    override string repr() { result = "an open file" }
+
+
+}
+
+class OpenFileConfiguration extends TaintTracking::Configuration {
+
+    OpenFileConfiguration() {  this = "Open file configuration" }
+
+    override predicate isSource(DataFlow::Node src, TaintKind kind) {
+        theOpenFunction().(FunctionObject).getACall() = src.asCfgNode() and
+        kind instanceof OpenFile
+    }
+
+    override predicate isSink(DataFlow::Node sink, TaintKind kind) {
+        none()
+    }
+
+}

python/ql/src/semmle/python/objects/Modules.qll

@@ -435,8 +435,11 @@ class AbsentModuleAttributeObjectInternal extends ObjectInternal, TAbsentModuleA
 
     override predicate subscriptUnknown() { any() }
 
-    /* We know what this is called, but not its innate name */
-    override string getName() { none() }
+    /* We know what this is called, but not its innate name.
+     * However, if we are looking for things by name, this is a reasonable approximation */
+    override string getName() {
+        this = TAbsentModuleAttribute(_, result)
+    }
 
     override predicate contextSensitiveCallee() { none() }
 

python/ql/src/semmle/python/security/ClearText.qll

@@ -0,0 +1,66 @@
+import python
+import semmle.python.security.TaintTracking
+import semmle.python.security.SensitiveData
+import semmle.python.dataflow.Files
+import semmle.python.web.Http
+
+module ClearTextStorage {
+
+    abstract class Sink extends TaintSink {
+        override predicate sinks(TaintKind kind) {
+            kind instanceof SensitiveData
+        }
+    }
+
+    class CookieStorageSink extends Sink {
+        CookieStorageSink() {
+            any(CookieSet cookie).getValue() = this
+        }
+    }
+
+    class FileStorageSink extends Sink {
+        FileStorageSink() {
+            exists(CallNode call, AttrNode meth, string name |
+                any(OpenFile fd).taints(meth.getObject(name)) and
+                call.getFunction() = meth and
+                call.getAnArg() = this |
+                name = "write"
+            )
+        }
+    }
+
+}
+
+module ClearTextLogging {
+
+    abstract class Sink extends TaintSink {
+        override predicate sinks(TaintKind kind) {
+            kind instanceof SensitiveData
+        }
+    }
+
+    class PrintSink extends Sink {
+        PrintSink() {
+            exists(CallNode call |
+                call.getAnArg() = this and
+                thePrintFunction().(FunctionObject).getACall() = call
+            )
+        }
+    }
+
+    class LoggingSink extends Sink {
+        LoggingSink() {
+            exists(CallNode call, AttrNode meth, string name |
+                call.getFunction() = meth and
+                meth.getObject(name).(NameNode).getId().matches("logg%") and
+                call.getAnArg() = this |
+                name = "error" or
+                name = "warn" or
+                name = "warning" or
+                name = "debug" or
+                name = "info"
+            )
+        }
+    }
+
+}