Merge branch 'main' of https://github.com/github/codeql into python-a-few-minor-cleanups

This commit is contained in:
Taus
2021-06-18 17:38:43 +00:00
committed by GitHub
655 changed files with 17448 additions and 1701 deletions

View File

@@ -6,7 +6,7 @@
* correctness
* security/cwe/cwe-78
* @problem.severity error
* @security-severity 5.9
* @security-severity 9.8
* @sub-severity high
* @precision high
* @id py/use-of-input

View File

@@ -6,7 +6,7 @@
* @tags security
* external/cwe/cwe-200
* @problem.severity error
* @security-severity 3.6
* @security-severity 6.5
* @sub-severity low
* @precision high
* @id py/bind-socket-all-network-interfaces

View File

@@ -5,7 +5,7 @@
* @kind path-problem
* @precision low
* @problem.severity error
* @security-severity 5.9
* @security-severity 7.8
* @tags security external/cwe/cwe-20
*/

View File

@@ -3,7 +3,7 @@
* @description Matching a URL or hostname against a regular expression that contains an unescaped dot as part of the hostname might match more hostnames than expected.
* @kind problem
* @problem.severity warning
* @security-severity 5.9
* @security-severity 7.8
* @precision high
* @id py/incomplete-hostname-regexp
* @tags correctness

View File

@@ -3,7 +3,7 @@
* @description Security checks on the substrings of an unparsed URL are often vulnerable to bypassing.
* @kind problem
* @problem.severity warning
* @security-severity 5.9
* @security-severity 7.8
* @precision high
* @id py/incomplete-url-substring-sanitization
* @tags correctness

View File

@@ -3,7 +3,7 @@
* @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
* @kind path-problem
* @problem.severity error
* @security-severity 6.4
* @security-severity 7.5
* @sub-severity high
* @precision high
* @id py/path-injection

View File

@@ -6,7 +6,7 @@
* @kind path-problem
* @id py/tarslip
* @problem.severity error
* @security-severity 6.4
* @security-severity 7.5
* @precision medium
* @tags security
* external/cwe/cwe-022

View File

@@ -4,7 +4,7 @@
* user to change the meaning of the command.
* @kind path-problem
* @problem.severity error
* @security-severity 5.9
* @security-severity 9.8
* @sub-severity high
* @precision high
* @id py/command-line-injection

View File

@@ -4,7 +4,7 @@
* cause a cross-site scripting vulnerability.
* @kind problem
* @problem.severity error
* @security-severity 2.9
* @security-severity 6.1
* @precision medium
* @id py/jinja2/autoescape-false
* @tags security

View File

@@ -4,7 +4,7 @@
* allows for a cross-site scripting vulnerability.
* @kind path-problem
* @problem.severity error
* @security-severity 2.9
* @security-severity 6.1
* @sub-severity high
* @precision high
* @id py/reflective-xss

View File

@@ -4,7 +4,7 @@
* malicious SQL code by the user.
* @kind path-problem
* @problem.severity error
* @security-severity 6.4
* @security-severity 8.8
* @precision high
* @id py/sql-injection
* @tags security

View File

@@ -4,7 +4,7 @@
* code execution.
* @kind path-problem
* @problem.severity error
* @security-severity 10.0
* @security-severity 9.3
* @sub-severity high
* @precision high
* @id py/code-injection

View File

@@ -5,7 +5,7 @@
* developing a subsequent exploit.
* @kind path-problem
* @problem.severity error
* @security-severity 3.6
* @security-severity 5.4
* @precision high
* @id py/stack-trace-exposure
* @tags security

View File

@@ -3,7 +3,7 @@
* @description Running a Flask app in debug mode may allow an attacker to run arbitrary code through the Werkzeug debugger.
* @kind problem
* @problem.severity error
* @security-severity 6.4
* @security-severity 7.5
* @precision high
* @id py/flask-debug
* @tags security

View File

@@ -3,7 +3,7 @@
* @description Accepting unknown host keys can allow man-in-the-middle attacks.
* @kind problem
* @problem.severity error
* @security-severity 5.2
* @security-severity 7.5
* @precision high
* @id py/paramiko-missing-host-key-validation
* @tags security

View File

@@ -3,7 +3,7 @@
* @description Making a request without certificate validation can allow man-in-the-middle attacks.
* @kind problem
* @problem.severity error
* @security-severity 5.2
* @security-severity 7.5
* @precision medium
* @id py/request-without-cert-validation
* @tags security

View File

@@ -4,7 +4,7 @@
* expose it to an attacker.
* @kind path-problem
* @problem.severity error
* @security-severity 5.9
* @security-severity 7.5
* @precision high
* @id py/clear-text-logging-sensitive-data
* @tags security

View File

@@ -4,7 +4,7 @@
* attacker.
* @kind path-problem
* @problem.severity error
* @security-severity 5.9
* @security-severity 7.5
* @precision high
* @id py/clear-text-storage-sensitive-data
* @tags security

View File

@@ -3,7 +3,7 @@
* @description Use of a cryptographic key that is too small may allow the encryption to be broken.
* @kind problem
* @problem.severity error
* @security-severity 5.2
* @security-severity 7.5
* @precision high
* @id py/weak-crypto-key
* @tags security

View File

@@ -3,7 +3,7 @@
* @description Using broken or weak cryptographic algorithms can compromise security.
* @kind problem
* @problem.severity warning
* @security-severity 5.2
* @security-severity 7.5
* @precision high
* @id py/weak-cryptographic-algorithm
* @tags security

View File

@@ -5,7 +5,7 @@
* @id py/insecure-default-protocol
* @kind problem
* @problem.severity warning
* @security-severity 5.2
* @security-severity 7.5
* @precision high
* @tags security
* external/cwe/cwe-327

View File

@@ -4,7 +4,7 @@
* @id py/insecure-protocol
* @kind problem
* @problem.severity warning
* @security-severity 5.2
* @security-severity 7.5
* @precision high
* @tags security
* external/cwe/cwe-327

View File

@@ -3,7 +3,7 @@
* @description Using broken or weak cryptographic hashing algorithms can compromise security.
* @kind path-problem
* @problem.severity warning
* @security-severity 5.9
* @security-severity 7.5
* @precision high
* @id py/weak-sensitive-data-hashing
* @tags security

View File

@@ -4,7 +4,7 @@
* @kind problem
* @id py/insecure-temporary-file
* @problem.severity error
* @security-severity 5.9
* @security-severity 7.0
* @sub-severity high
* @precision high
* @tags external/cwe/cwe-377

View File

@@ -4,7 +4,7 @@
* @kind path-problem
* @id py/unsafe-deserialization
* @problem.severity error
* @security-severity 5.9
* @security-severity 9.8
* @sub-severity high
* @precision high
* @tags external/cwe/cwe-502

View File

@@ -4,7 +4,7 @@
* may cause redirection to malicious web sites.
* @kind path-problem
* @problem.severity error
* @security-severity 2.7
* @security-severity 6.1
* @sub-severity low
* @id py/url-redirection
* @tags security

View File

@@ -4,7 +4,7 @@
* @kind problem
* @id py/overly-permissive-file
* @problem.severity warning
* @security-severity 5.9
* @security-severity 7.8
* @sub-severity high
* @precision medium
* @tags external/cwe/cwe-732

View File

@@ -3,7 +3,7 @@
* @description Credentials are hard coded in the source code of the application.
* @kind path-problem
* @problem.severity error
* @security-severity 5.9
* @security-severity 9.8
* @precision medium
* @id py/hardcoded-credentials
* @tags security

View File

@@ -1,28 +0,0 @@
from django.conf.urls import url
from clickhouse_driver import Client
from clickhouse_driver import connect
from aioch import Client as aiochClient
class MyClient(Client):
def dummy(self):
return None
def show_user(request, username):
# BAD -- Untrusted user input is directly injected into the sql query using async library 'aioch'
aclient = aiochClient("localhost")
progress = await aclient.execute_with_progress("SELECT * FROM users WHERE username = '%s'" % username)
# BAD -- Untrusted user input is directly injected into the sql query using native client of library 'clickhouse_driver'
Client('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
# GOOD -- query uses prepared statements
query = "SELECT * FROM users WHERE username = %(username)s"
Client('localhost').execute(query, {"username": username})
# BAD -- PEP249 interface
conn = connect('clickhouse://localhost')
cursor = conn.cursor()
cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
urlpatterns = [url(r'^users/(?P<username>[^/]+)$', show_user)]

View File

@@ -1,59 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
If a database query (such as a SQL or NoSQL query) is built from
user-provided data without sufficient sanitization, a user
may be able to run malicious database queries.
</p>
</overview>
<recommendation>
<p>
Most database connector libraries offer a way of safely
embedding untrusted data into a query by means of query parameters
or prepared statements.
</p>
</recommendation>
<example>
<p>
In the following snippet, a user is fetched from a <code>ClickHouse</code> database
using five different queries. In the "BAD" cases the query is built directly from user-controlled data.
In the "GOOD" case the user-controlled data is safely embedded into the query by using query parameters.
</p>
<p>
In the first case, the query executed via aioch Client. aioch - is a module
for asynchronous queries to database.
</p>
<p>
In the second and third cases, the connection is established via `Client` class.
This class implement different method to execute a query.
</p>
<p>
In the forth case, good pattern is presented. Query parameters are send through
second dict-like argument.
</p>
<p>
In the fifth case, there is example of PEP249 interface usage.
</p>
<p>
In the sixth case, there is custom Class usge which is a subclass of default Client.
</p>
<sample src="ClickHouseSQLInjection.py" />
</example>
<references>
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/SQL_injection">SQL injection</a>.</li>
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html">SQL Injection Prevention Cheat Sheet</a>.</li>
</references>
</qhelp>

View File

@@ -1,22 +0,0 @@
/**
* @id py/yandex/clickhouse-sql-injection
* @name Clickhouse SQL query built from user-controlled sources
* @description Building a SQL query from user-controlled sources is vulnerable to insertion of
* malicious SQL code by the user.
* @kind path-problem
* @problem.severity error
* @precision high
* @tags security
* external/cwe/cwe-089
* external/owasp/owasp-a1
*/
import python
import experimental.semmle.python.frameworks.ClickHouseDriver
import semmle.python.security.dataflow.SqlInjection
import DataFlow::PathGraph
from SQLInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "This SQL query depends on $@.", source.getNode(),
"a user-provided value"

View File

@@ -97,6 +97,11 @@ module API {
*/
Node getASubclass() { result = getASuccessor(Label::subclass()) }
/**
* Gets a node representing the result from awaiting this node.
*/
Node getAwaited() { result = getASuccessor(Label::await()) }
/**
* Gets a string representation of the lexicographically least among all shortest access paths
* from the root to this node.
@@ -469,6 +474,14 @@ module API {
exists(DataFlow::Node superclass | pred.flowsTo(superclass) |
ref.asExpr().(ClassExpr).getABase() = superclass.asExpr()
)
or
// awaiting
exists(Await await, DataFlow::Node awaitedValue |
lbl = Label::await() and
ref.asExpr() = await and
await.getValue() = awaitedValue.asExpr() and
pred.flowsTo(awaitedValue)
)
)
or
// Built-ins, treated as members of the module `builtins`
@@ -585,5 +598,9 @@ private module Label {
/** Gets the `return` edge label. */
string return() { result = "getReturn()" }
/** Gets the `subclass` edge label. */
string subclass() { result = "getASubclass()" }
/** Gets the `await` edge label. */
string await() { result = "getAwaited()" }
}

View File

@@ -4,7 +4,9 @@
// If you add modeling of a new framework/library, remember to add it it to the docs in
// `docs/codeql/support/reusables/frameworks.rst`
private import semmle.python.frameworks.Aioch
private import semmle.python.frameworks.Aiohttp
private import semmle.python.frameworks.ClickhouseDriver
private import semmle.python.frameworks.Cryptodome
private import semmle.python.frameworks.Cryptography
private import semmle.python.frameworks.Dill

View File

@@ -168,7 +168,13 @@ module Consistency {
msg = "ArgumentNode is missing PostUpdateNode."
}
query predicate postWithInFlow(PostUpdateNode n, string msg) {
// This predicate helps the compiler forget that in some languages
// it is impossible for a `PostUpdateNode` to be the target of
// `simpleLocalFlowStep`.
private predicate isPostUpdateNode(Node n) { n instanceof PostUpdateNode or none() }
query predicate postWithInFlow(Node n, string msg) {
isPostUpdateNode(n) and
simpleLocalFlowStep(_, n) and
msg = "PostUpdateNode should not be the target of local flow."
}

View File

@@ -23,15 +23,57 @@ class OptionalContentName extends string {
OptionalContentName() { this instanceof ContentName or this = "" }
}
/**
* A description of a step on an inter-procedural data flow path.
*/
private newtype TStepSummary =
LevelStep() or
CallStep() or
ReturnStep() or
StoreStep(ContentName content) or
LoadStep(ContentName content)
cached
private module Cached {
/**
* A description of a step on an inter-procedural data flow path.
*/
cached
newtype TStepSummary =
LevelStep() or
CallStep() or
ReturnStep() or
StoreStep(ContentName content) or
LoadStep(ContentName content)
/** Gets the summary resulting from appending `step` to type-tracking summary `tt`. */
cached
TypeTracker append(TypeTracker tt, StepSummary step) {
exists(Boolean hasCall, OptionalContentName content | tt = MkTypeTracker(hasCall, content) |
step = LevelStep() and result = tt
or
step = CallStep() and result = MkTypeTracker(true, content)
or
step = ReturnStep() and hasCall = false and result = tt
or
step = LoadStep(content) and result = MkTypeTracker(hasCall, "")
or
exists(string p | step = StoreStep(p) and content = "" and result = MkTypeTracker(hasCall, p))
)
}
/**
* Gets the summary that corresponds to having taken a forwards
* heap and/or intra-procedural step from `nodeFrom` to `nodeTo`.
*
* Steps contained in this predicate should _not_ depend on the call graph.
*/
cached
predicate stepNoCall(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
exists(Node mid | nodeFrom.flowsTo(mid) and smallstepNoCall(mid, nodeTo, summary))
}
/**
* Gets the summary that corresponds to having taken a forwards
* inter-procedural step from `nodeFrom` to `nodeTo`.
*/
cached
predicate stepCall(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
exists(Node mid | nodeFrom.flowsTo(mid) and smallstepCall(mid, nodeTo, summary))
}
}
private import Cached
/**
* INTERNAL: Use `TypeTracker` or `TypeBackTracker` instead.
@@ -53,28 +95,29 @@ class StepSummary extends TStepSummary {
}
}
pragma[noinline]
private predicate smallstepNoCall(Node nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
jumpStep(nodeFrom, nodeTo) and
summary = LevelStep()
or
exists(string content |
StepSummary::localSourceStoreStep(nodeFrom, nodeTo, content) and
summary = StoreStep(content)
or
basicLoadStep(nodeFrom, nodeTo, content) and summary = LoadStep(content)
)
}
pragma[noinline]
private predicate smallstepCall(Node nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
callStep(nodeFrom, nodeTo) and summary = CallStep()
or
returnStep(nodeFrom, nodeTo) and
summary = ReturnStep()
}
/** Provides predicates for updating step summaries (`StepSummary`s). */
module StepSummary {
/**
* Gets the summary that corresponds to having taken a forwards
* heap and/or intra-procedural step from `nodeFrom` to `nodeTo`.
*
* Steps contained in this predicate should _not_ depend on the call graph.
*/
cached
private predicate stepNoCall(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
exists(Node mid | nodeFrom.flowsTo(mid) and smallstepNoCall(mid, nodeTo, summary))
}
/**
* Gets the summary that corresponds to having taken a forwards
* inter-procedural step from `nodeFrom` to `nodeTo`.
*/
cached
private predicate stepCall(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
exists(Node mid | nodeFrom.flowsTo(mid) and smallstepCall(mid, nodeTo, summary))
}
/**
* Gets the summary that corresponds to having taken a forwards
* heap and/or inter-procedural step from `nodeFrom` to `nodeTo`.
@@ -92,27 +135,6 @@ module StepSummary {
stepCall(nodeFrom, nodeTo, summary)
}
pragma[noinline]
private predicate smallstepNoCall(Node nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
jumpStep(nodeFrom, nodeTo) and
summary = LevelStep()
or
exists(string content |
localSourceStoreStep(nodeFrom, nodeTo, content) and
summary = StoreStep(content)
or
basicLoadStep(nodeFrom, nodeTo, content) and summary = LoadStep(content)
)
}
pragma[noinline]
private predicate smallstepCall(Node nodeFrom, LocalSourceNode nodeTo, StepSummary summary) {
callStep(nodeFrom, nodeTo) and summary = CallStep()
or
returnStep(nodeFrom, nodeTo) and
summary = ReturnStep()
}
/**
* Gets the summary that corresponds to having taken a forwards
* local, heap and/or inter-procedural step from `nodeFrom` to `nodeTo`.
@@ -193,18 +215,7 @@ class TypeTracker extends TTypeTracker {
TypeTracker() { this = MkTypeTracker(hasCall, content) }
/** Gets the summary resulting from appending `step` to this type-tracking summary. */
cached
TypeTracker append(StepSummary step) {
step = LevelStep() and result = this
or
step = CallStep() and result = MkTypeTracker(true, content)
or
step = ReturnStep() and hasCall = false and result = this
or
step = LoadStep(content) and result = MkTypeTracker(hasCall, "")
or
exists(string p | step = StoreStep(p) and content = "" and result = MkTypeTracker(hasCall, p))
}
TypeTracker append(StepSummary step) { result = append(this, step) }
/** Gets a textual representation of this summary. */
string toString() {

View File

@@ -0,0 +1,52 @@
/**
* Provides classes modeling security-relevant aspects of the `aioch` PyPI package (an
* async-io version of the `clickhouse-driver` PyPI package).
*
* See https://pypi.org/project/aioch/
*/
private import python
private import semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.PEP249
private import semmle.python.frameworks.ClickhouseDriver
/**
* INTERNAL: Do not use.
*
* Provides models for `aioch` PyPI package (an async-io version of the
* `clickhouse-driver` PyPI package).
*
* See https://pypi.org/project/aioch/
*/
module Aioch {
/** Provides models for `aioch.Client` class and subclasses. */
module Client {
/** Gets a reference to the `aioch.Client` class or any subclass. */
API::Node subclassRef() {
result = API::moduleImport("aioch").getMember("Client").getASubclass*()
}
/** Gets a reference to an instance of `clickhouse_driver.Client` or any subclass. */
API::Node instance() { result = subclassRef().getReturn() }
}
/**
* A call to any of the the execute methods on a `aioch.Client`, which are just async
* versions of the methods in the `clickhouse-driver` PyPI package.
*
* See
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_iter
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_with_progress
*/
class ClientExecuteCall extends SqlExecution::Range, DataFlow::CallCfgNode {
ClientExecuteCall() {
exists(string methodName | methodName = ClickhouseDriver::getExecuteMethodName() |
this = Client::instance().getMember(methodName).getACall()
)
}
override DataFlow::Node getSql() { result in [this.getArg(0), this.getArgByName("query")] }
}
}

View File

@@ -0,0 +1,65 @@
/**
* Provides classes modeling security-relevant aspects of the `clickhouse-driver` PyPI package.
* See
* - https://pypi.org/project/clickhouse-driver/
* - https://clickhouse-driver.readthedocs.io/en/latest/
*/
private import python
private import semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.PEP249
/**
* INTERNAL: Do not use.
*
* Provides models for `clickhouse-driver` PyPI package (imported as `clickhouse_driver`).
* See
* - https://pypi.org/project/clickhouse-driver/
* - https://clickhouse-driver.readthedocs.io/en/latest/
*/
module ClickhouseDriver {
/**
* `clickhouse_driver` implements PEP249,
* providing ways to execute SQL statements against a database.
*/
class ClickHouseDriverPEP249 extends PEP249ModuleApiNode {
ClickHouseDriverPEP249() { this = API::moduleImport("clickhouse_driver") }
}
/** Provides models for `clickhouse_driver.Client` class and subclasses. */
module Client {
/** Gets a reference to the `clickhouse_driver.Client` class or any subclass. */
API::Node subclassRef() {
exists(API::Node classRef |
// canonical definition
classRef = API::moduleImport("clickhouse_driver").getMember("client").getMember("Client")
or
// commonly used alias
classRef = API::moduleImport("clickhouse_driver").getMember("Client")
|
result = classRef.getASubclass*()
)
}
/** Gets a reference to an instance of `clickhouse_driver.Client` or any subclass. */
API::Node instance() { result = subclassRef().getReturn() }
}
/** `clickhouse_driver.Client` execute method names */
string getExecuteMethodName() { result in ["execute_with_progress", "execute", "execute_iter"] }
/**
* A call to any of the the execute methods on a `clickhouse_driver.Client` method
*
* See
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_iter
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_with_progress
*/
class ClientExecuteCall extends SqlExecution::Range, DataFlow::CallCfgNode {
ClientExecuteCall() { this = Client::instance().getMember(getExecuteMethodName()).getACall() }
override DataFlow::Node getSql() { result in [this.getArg(0), this.getArgByName("query")] }
}
}