hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 18:55:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Python: Simple port of URL redirect query

Browse Source 
Still have not added sanitizer, but seems like old sanitizer was a bit too broad
(also covering %-formatting)
This commit is contained in:
Rasmus Wriedt Larsen
2021-01-19 15:44:51 +01:00
parent 9d8925ae6a
commit d8bfa3565f
6 changed files with 122 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
python/change-notes/2021-01-19-port-url-redirect-query.md Normal file
View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* Ported URL redirection (`py/url-redirection`) query to use new data-flow library. This might result in different results, but overall a more robust and accurate analysis.

32
python/ql/src/Security/CWE-601/UrlRedirect.ql
View File

@@ -12,30 +12,10 @@
*/
import python
import semmle.python.security.Paths
import semmle.python.web.HttpRedirect
import semmle.python.web.HttpRequest
import semmle.python.security.strings.Untrusted
import semmle.python.security.dataflow.UrlRedirect
import DataFlow::PathGraph
/** Url redirection is a problem only if the user controls the prefix of the URL */
class UntrustedPrefixStringKind extends UntrustedStringKind {
override TaintKind getTaintForFlowStep(ControlFlowNode fromnode, ControlFlowNode tonode) {
result = UntrustedStringKind.super.getTaintForFlowStep(fromnode, tonode) and
not tonode.(BinaryExprNode).getRight() = fromnode
}
}
class UrlRedirectConfiguration extends TaintTracking::Configuration {
UrlRedirectConfiguration() { this = "URL redirect configuration" }
override predicate isSource(TaintTracking::Source source) {
source instanceof HttpRequestTaintSource
}
override predicate isSink(TaintTracking::Sink sink) { sink instanceof HttpRedirectTaintSink }
}
from UrlRedirectConfiguration config, TaintedPathSource src, TaintedPathSink sink
where config.hasFlowPath(src, sink)
select sink.getSink(), src, sink, "Untrusted URL redirection due to $@.", src.getSource(),
"a user-provided value"
from UrlRedirectConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Untrusted URL redirection due to $@.", source.getNode(),
"A user-provided value"

41
python/ql/src/experimental/Security-old-dataflow/CWE-601/UrlRedirect.ql Normal file
View File

@@ -0,0 +1,41 @@
/**
* @name URL redirection from remote source
* @description URL redirection based on unvalidated user input
* may cause redirection to malicious web sites.
* @kind path-problem
* @problem.severity error
* @sub-severity low
* @id py/url-redirection
* @tags security
* external/cwe/cwe-601
* @precision high
*/
import python
import semmle.python.security.Paths
import semmle.python.web.HttpRedirect
import semmle.python.web.HttpRequest
import semmle.python.security.strings.Untrusted
/** Url redirection is a problem only if the user controls the prefix of the URL */
class UntrustedPrefixStringKind extends UntrustedStringKind {
override TaintKind getTaintForFlowStep(ControlFlowNode fromnode, ControlFlowNode tonode) {
result = UntrustedStringKind.super.getTaintForFlowStep(fromnode, tonode) and
not tonode.(BinaryExprNode).getRight() = fromnode
}
}
class UrlRedirectConfiguration extends TaintTracking::Configuration {
UrlRedirectConfiguration() { this = "URL redirect configuration" }
override predicate isSource(TaintTracking::Source source) {
source instanceof HttpRequestTaintSource
}
override predicate isSink(TaintTracking::Sink sink) { sink instanceof HttpRedirectTaintSink }
}
from UrlRedirectConfiguration config, TaintedPathSource src, TaintedPathSink sink
where config.hasFlowPath(src, sink)
select sink.getSink(), src, sink, "Untrusted URL redirection due to $@.", src.getSource(),
"a user-provided value"

28
python/ql/src/semmle/python/security/dataflow/UrlRedirect.qll Normal file
View File

@@ -0,0 +1,28 @@
/**
* Provides a taint-tracking configuration for detecting URL redirection
* vulnerabilities.
*/
import python
import semmle.python.dataflow.new.DataFlow
import semmle.python.dataflow.new.TaintTracking
import semmle.python.Concepts
import semmle.python.dataflow.new.RemoteFlowSources
import semmle.python.dataflow.new.BarrierGuards
/**
* A taint-tracking configuration for detecting URL redirection vulnerabilities.
*/
class UrlRedirectConfiguration extends TaintTracking::Configuration {
UrlRedirectConfiguration() { this = "UrlRedirectConfiguration" }
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) {
sink = any(HTTP::Server::HttpRedirectResponse e).getRedirectLocation()
}
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
guard instanceof StringConstCompare
}
}

91
python/ql/test/query-tests/Security/CWE-601/UrlRedirect.expected
View File

@@ -1,52 +1,43 @@
edges
| test.py:7:14:7:25 | dict of externally controlled string | test.py:7:14:7:43 | externally controlled string |
| test.py:7:14:7:25 | dict of externally controlled string | test.py:7:14:7:43 | externally controlled string |
| test.py:7:14:7:43 | externally controlled string | test.py:8:21:8:26 | externally controlled string |
| test.py:7:14:7:43 | externally controlled string | test.py:8:21:8:26 | externally controlled string |
| test.py:30:17:30:28 | dict of externally controlled string | test.py:30:17:30:46 | externally controlled string |
| test.py:30:17:30:28 | dict of externally controlled string | test.py:30:17:30:46 | externally controlled string |
| test.py:30:17:30:46 | externally controlled string | test.py:31:41:31:49 | externally controlled string |
| test.py:30:17:30:46 | externally controlled string | test.py:31:41:31:49 | externally controlled string |
| test.py:31:12:31:50 | externally controlled string | test.py:32:21:32:24 | externally controlled string |
| test.py:31:12:31:50 | externally controlled string | test.py:32:21:32:24 | externally controlled string |
| test.py:31:41:31:49 | externally controlled string | test.py:31:12:31:50 | externally controlled string |
| test.py:31:41:31:49 | externally controlled string | test.py:31:12:31:50 | externally controlled string |
| test.py:37:17:37:28 | dict of externally controlled string | test.py:37:17:37:46 | externally controlled string |
| test.py:37:17:37:28 | dict of externally controlled string | test.py:37:17:37:46 | externally controlled string |
| test.py:37:17:37:46 | externally controlled string | test.py:38:32:38:40 | externally controlled string |
| test.py:37:17:37:46 | externally controlled string | test.py:38:32:38:40 | externally controlled string |
| test.py:38:12:38:42 | externally controlled string | test.py:39:21:39:24 | externally controlled string |
| test.py:38:12:38:42 | externally controlled string | test.py:39:21:39:24 | externally controlled string |
| test.py:38:32:38:40 | externally controlled string | test.py:38:12:38:42 | externally controlled string |
| test.py:38:32:38:40 | externally controlled string | test.py:38:12:38:42 | externally controlled string |
| test.py:53:17:53:28 | dict of externally controlled string | test.py:53:17:53:46 | externally controlled string |
| test.py:53:17:53:28 | dict of externally controlled string | test.py:53:17:53:46 | externally controlled string |
| test.py:53:17:53:46 | externally controlled string | test.py:54:14:54:22 | externally controlled string |
| test.py:53:17:53:46 | externally controlled string | test.py:54:14:54:22 | externally controlled string |
| test.py:54:14:54:22 | externally controlled string | test.py:54:14:54:41 | externally controlled string |
| test.py:54:14:54:22 | externally controlled string | test.py:54:14:54:41 | externally controlled string |
| test.py:54:14:54:41 | externally controlled string | test.py:55:21:55:26 | externally controlled string |
| test.py:54:14:54:41 | externally controlled string | test.py:55:21:55:26 | externally controlled string |
| test.py:60:17:60:28 | dict of externally controlled string | test.py:60:17:60:46 | externally controlled string |
| test.py:60:17:60:28 | dict of externally controlled string | test.py:60:17:60:46 | externally controlled string |
| test.py:60:17:60:46 | externally controlled string | test.py:61:40:61:48 | externally controlled string |
| test.py:60:17:60:46 | externally controlled string | test.py:61:40:61:48 | externally controlled string |
| test.py:61:14:61:49 | externally controlled string | test.py:62:21:62:26 | externally controlled string |
| test.py:61:14:61:49 | externally controlled string | test.py:62:21:62:26 | externally controlled string |
| test.py:61:40:61:48 | externally controlled string | test.py:61:14:61:49 | externally controlled string |
| test.py:61:40:61:48 | externally controlled string | test.py:61:14:61:49 | externally controlled string |
| test.py:67:17:67:28 | dict of externally controlled string | test.py:67:17:67:46 | externally controlled string |
| test.py:67:17:67:28 | dict of externally controlled string | test.py:67:17:67:46 | externally controlled string |
| test.py:67:17:67:46 | externally controlled string | test.py:68:17:68:25 | externally controlled string |
| test.py:67:17:67:46 | externally controlled string | test.py:68:17:68:25 | externally controlled string |
| test.py:68:14:68:41 | externally controlled string | test.py:69:21:69:26 | externally controlled string |
| test.py:68:14:68:41 | externally controlled string | test.py:69:21:69:26 | externally controlled string |
| test.py:68:17:68:25 | externally controlled string | test.py:68:14:68:41 | externally controlled string |
| test.py:68:17:68:25 | externally controlled string | test.py:68:14:68:41 | externally controlled string |
| test.py:7:14:7:25 | ControlFlowNode for Attribute | test.py:8:21:8:26 | ControlFlowNode for target |
| test.py:15:17:15:28 | ControlFlowNode for Attribute | test.py:18:21:18:24 | ControlFlowNode for safe |
| test.py:23:17:23:28 | ControlFlowNode for Attribute | test.py:25:21:25:24 | ControlFlowNode for safe |
| test.py:30:17:30:28 | ControlFlowNode for Attribute | test.py:32:21:32:24 | ControlFlowNode for safe |
| test.py:37:17:37:28 | ControlFlowNode for Attribute | test.py:39:21:39:24 | ControlFlowNode for safe |
| test.py:44:17:44:28 | ControlFlowNode for Attribute | test.py:46:21:46:24 | ControlFlowNode for safe |
| test.py:53:17:53:28 | ControlFlowNode for Attribute | test.py:55:21:55:26 | ControlFlowNode for unsafe |
| test.py:60:17:60:28 | ControlFlowNode for Attribute | test.py:62:21:62:26 | ControlFlowNode for unsafe |
| test.py:67:17:67:28 | ControlFlowNode for Attribute | test.py:69:21:69:26 | ControlFlowNode for unsafe |
| test.py:74:17:74:28 | ControlFlowNode for Attribute | test.py:76:21:76:26 | ControlFlowNode for unsafe |
nodes
| test.py:7:14:7:25 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:8:21:8:26 | ControlFlowNode for target | semmle.label | ControlFlowNode for target |
| test.py:15:17:15:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:18:21:18:24 | ControlFlowNode for safe | semmle.label | ControlFlowNode for safe |
| test.py:23:17:23:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:25:21:25:24 | ControlFlowNode for safe | semmle.label | ControlFlowNode for safe |
| test.py:30:17:30:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:32:21:32:24 | ControlFlowNode for safe | semmle.label | ControlFlowNode for safe |
| test.py:37:17:37:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:39:21:39:24 | ControlFlowNode for safe | semmle.label | ControlFlowNode for safe |
| test.py:44:17:44:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:46:21:46:24 | ControlFlowNode for safe | semmle.label | ControlFlowNode for safe |
| test.py:53:17:53:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:55:21:55:26 | ControlFlowNode for unsafe | semmle.label | ControlFlowNode for unsafe |
| test.py:60:17:60:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:62:21:62:26 | ControlFlowNode for unsafe | semmle.label | ControlFlowNode for unsafe |
| test.py:67:17:67:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:69:21:69:26 | ControlFlowNode for unsafe | semmle.label | ControlFlowNode for unsafe |
| test.py:74:17:74:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:76:21:76:26 | ControlFlowNode for unsafe | semmle.label | ControlFlowNode for unsafe |
#select
| test.py:8:21:8:26 | target | test.py:7:14:7:25 | dict of externally controlled string | test.py:8:21:8:26 | externally controlled string | Untrusted URL redirection due to $@. | test.py:7:14:7:25 | Attribute | a user-provided value |
| test.py:32:21:32:24 | safe | test.py:30:17:30:28 | dict of externally controlled string | test.py:32:21:32:24 | externally controlled string | Untrusted URL redirection due to $@. | test.py:30:17:30:28 | Attribute | a user-provided value |
| test.py:39:21:39:24 | safe | test.py:37:17:37:28 | dict of externally controlled string | test.py:39:21:39:24 | externally controlled string | Untrusted URL redirection due to $@. | test.py:37:17:37:28 | Attribute | a user-provided value |
| test.py:55:21:55:26 | unsafe | test.py:53:17:53:28 | dict of externally controlled string | test.py:55:21:55:26 | externally controlled string | Untrusted URL redirection due to $@. | test.py:53:17:53:28 | Attribute | a user-provided value |
| test.py:62:21:62:26 | unsafe | test.py:60:17:60:28 | dict of externally controlled string | test.py:62:21:62:26 | externally controlled string | Untrusted URL redirection due to $@. | test.py:60:17:60:28 | Attribute | a user-provided value |
| test.py:69:21:69:26 | unsafe | test.py:67:17:67:28 | dict of externally controlled string | test.py:69:21:69:26 | externally controlled string | Untrusted URL redirection due to $@. | test.py:67:17:67:28 | Attribute | a user-provided value |
| test.py:8:21:8:26 | ControlFlowNode for target | test.py:7:14:7:25 | ControlFlowNode for Attribute | test.py:8:21:8:26 | ControlFlowNode for target | Untrusted URL redirection due to $@. | test.py:7:14:7:25 | ControlFlowNode for Attribute | A user-provided value |
| test.py:18:21:18:24 | ControlFlowNode for safe | test.py:15:17:15:28 | ControlFlowNode for Attribute | test.py:18:21:18:24 | ControlFlowNode for safe | Untrusted URL redirection due to $@. | test.py:15:17:15:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:25:21:25:24 | ControlFlowNode for safe | test.py:23:17:23:28 | ControlFlowNode for Attribute | test.py:25:21:25:24 | ControlFlowNode for safe | Untrusted URL redirection due to $@. | test.py:23:17:23:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:32:21:32:24 | ControlFlowNode for safe | test.py:30:17:30:28 | ControlFlowNode for Attribute | test.py:32:21:32:24 | ControlFlowNode for safe | Untrusted URL redirection due to $@. | test.py:30:17:30:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:39:21:39:24 | ControlFlowNode for safe | test.py:37:17:37:28 | ControlFlowNode for Attribute | test.py:39:21:39:24 | ControlFlowNode for safe | Untrusted URL redirection due to $@. | test.py:37:17:37:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:46:21:46:24 | ControlFlowNode for safe | test.py:44:17:44:28 | ControlFlowNode for Attribute | test.py:46:21:46:24 | ControlFlowNode for safe | Untrusted URL redirection due to $@. | test.py:44:17:44:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:55:21:55:26 | ControlFlowNode for unsafe | test.py:53:17:53:28 | ControlFlowNode for Attribute | test.py:55:21:55:26 | ControlFlowNode for unsafe | Untrusted URL redirection due to $@. | test.py:53:17:53:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:62:21:62:26 | ControlFlowNode for unsafe | test.py:60:17:60:28 | ControlFlowNode for Attribute | test.py:62:21:62:26 | ControlFlowNode for unsafe | Untrusted URL redirection due to $@. | test.py:60:17:60:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:69:21:69:26 | ControlFlowNode for unsafe | test.py:67:17:67:28 | ControlFlowNode for Attribute | test.py:69:21:69:26 | ControlFlowNode for unsafe | Untrusted URL redirection due to $@. | test.py:67:17:67:28 | ControlFlowNode for Attribute | A user-provided value |
| test.py:76:21:76:26 | ControlFlowNode for unsafe | test.py:74:17:74:28 | ControlFlowNode for Attribute | test.py:76:21:76:26 | ControlFlowNode for unsafe | Untrusted URL redirection due to $@. | test.py:74:17:74:28 | ControlFlowNode for Attribute | A user-provided value |

8
python/ql/test/query-tests/Security/CWE-601/test.py
View File

@@ -15,14 +15,14 @@ def ok():
untrusted = request.args.get('target', '')
safe = "https://safe.com/"
safe += untrusted
return redirect(safe, code=302)
return redirect(safe, code=302) # FP
@app.route('/ok2')
def ok2():
untrusted = request.args.get('target', '')
safe = "https://safe.com/" + untrusted
return redirect(safe, code=302)
return redirect(safe, code=302) # FP
@app.route('/ok3')
@@ -43,7 +43,7 @@ def ok4():
def ok5():
untrusted = request.args.get('target', '')
safe = "https://safe.com/%s" % untrusted
return redirect(safe, code=302)
return redirect(safe, code=302) # FP
# Check that our sanitizer is not too broad
@@ -73,4 +73,4 @@ def not_ok3():
def not_ok4():
untrusted = request.args.get('target', '')
unsafe = "%s?login=success" % untrusted
return redirect(unsafe, code=302) # Missing result
return redirect(unsafe, code=302)