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

Merge pull request #11088 from yoff/python/inline-query-tests

Browse Source 
Python: Inline query tests
This commit is contained in:
yoff
2023-01-12 10:32:26 +01:00
committed by GitHub
parent 1d4b2fd0bb 03bd6cb414
commit 006eaf3e2a
10 changed files with 107 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
python/ql/test/experimental/dataflow/TestUtil/DataflowQueryTest.qll Normal file
View File

@@ -0,0 +1,58 @@
import python
import semmle.python.dataflow.new.DataFlow
import TestUtilities.InlineExpectationsTest
private import semmle.python.dataflow.new.internal.PrintNode
class DataFlowQueryTest extends InlineExpectationsTest {
DataFlowQueryTest() { this = "DataFlowQueryTest" }
override string getARelevantTag() { result = "result" }
override predicate hasActualResult(Location location, string element, string tag, string value) {
exists(DataFlow::Configuration cfg, DataFlow::Node sink | cfg.hasFlowTo(sink) |
location = sink.getLocation() and
tag = "result" and
value = "BAD" and
element = sink.toString()
)
}
// We allow annotating any sink with `result=OK` to signal
// safe sinks.
// Sometimes a line contains both an alert and a safe sink.
// In this situation, the annotation form `OK(safe sink)`
// can be useful.
override predicate hasOptionalResult(Location location, string element, string tag, string value) {
exists(DataFlow::Configuration cfg, DataFlow::Node sink |
cfg.isSink(sink) or cfg.isSink(sink, _)
|
location = sink.getLocation() and
tag = "result" and
value in ["OK", "OK(" + prettyNode(sink) + ")"] and
element = sink.toString()
)
}
}
query predicate missingAnnotationOnSink(Location location, string error, string element) {
error = "ERROR, you should add `# $ MISSING: result=BAD` or `result=OK` annotation" and
exists(DataFlow::Node sink |
exists(sink.getLocation().getFile().getRelativePath()) and
exists(DataFlow::Configuration cfg | cfg.isSink(sink) or cfg.isSink(sink, _)) and
location = sink.getLocation() and
element = prettyExpr(sink.asExpr()) and
not exists(DataFlow::Configuration cfg | cfg.hasFlowTo(sink)) and
not exists(FalseNegativeExpectation missingResult |
missingResult.getTag() = "result" and
missingResult.getValue() = "BAD" and
missingResult.getLocation().getFile() = location.getFile() and
missingResult.getLocation().getStartLine() = location.getStartLine()
) and
not exists(GoodExpectation okResult |
okResult.getTag() = "result" and
okResult.getValue() in ["OK", "OK(" + prettyNode(sink) + ")"] and
okResult.getLocation().getFile() = location.getFile() and
okResult.getLocation().getStartLine() = location.getStartLine()
)
)
}

2
python/ql/test/query-tests/Security/CWE-022-PathInjection/DataflowQueryTest.expected Normal file
View File

@@ -0,0 +1,2 @@
missingAnnotationOnSink
failures

3
python/ql/test/query-tests/Security/CWE-022-PathInjection/DataflowQueryTest.ql Normal file
View File

@@ -0,0 +1,3 @@
import python
import experimental.dataflow.TestUtil.DataflowQueryTest
import semmle.python.security.dataflow.PathInjectionQuery

4
python/ql/test/query-tests/Security/CWE-022-PathInjection/flask_path_injection.py
View File

@@ -10,7 +10,7 @@ STATIC_DIR = "/server/static/"
def download_file():
filename = request.args.get('filename', '')
# ok since `send_from_directory` ensure this stays within `STATIC_DIR`
return send_from_directory(STATIC_DIR, filename) # OK
return send_from_directory(STATIC_DIR, filename) # $result=OK
# see https://flask.palletsprojects.com/en/1.1.x/api/#flask.send_from_directory
@@ -18,4 +18,4 @@ def download_file():
def download_file():
dirname = request.args.get('dirname', '')
filename = request.args.get('filename', '')
return send_from_directory(dirname, filename) # NOT OK
return send_from_directory(dirname, filename) # $result=BAD result=OK(filename)

32
python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py
View File

@@ -10,7 +10,7 @@ STATIC_DIR = "/server/static/"
@app.route("/path1")
def path_injection():
filename = request.args.get('filename', '')
f = open(os.path.join(STATIC_DIR, filename)) # NOT OK
f = open(os.path.join(STATIC_DIR, filename)) # $result=BAD
@app.route("/path2")
@@ -18,7 +18,7 @@ def path_injection():
# Normalized, but not checked
filename = request.args.get('filename', '')
npath = os.path.normpath(os.path.join(STATIC_DIR, filename))
f = open(npath) # NOT OK
f = open(npath) # $result=BAD
@app.route("/path3")
@@ -28,7 +28,7 @@ def unsafe_path_normpath():
npath = os.path.normpath(os.path.join(STATIC_DIR, filename))
if npath.startswith(STATIC_DIR):
pass
f = open(npath) # NOT OK
f = open(npath) # $result=BAD
@app.route("/path4")
@@ -37,7 +37,7 @@ def safe_path_normpath():
filename = request.args.get('filename', '')
npath = os.path.normpath(os.path.join(STATIC_DIR, filename))
if npath.startswith(STATIC_DIR):
f = open(npath) # OK
f = open(npath) # $result=OK
@app.route("/path5")
@@ -45,7 +45,7 @@ def unsafe_path_realpath():
# Normalized (by `realpath` that also follows symlinks), but not checked properly
filename = request.args.get('filename', '')
npath = os.path.realpath(os.path.join(STATIC_DIR, filename))
f = open(npath) # NOT OK
f = open(npath) # $result=BAD
@app.route("/path6")
@@ -54,7 +54,7 @@ def safe_path_realpath():
filename = request.args.get('filename', '')
npath = os.path.realpath(os.path.join(STATIC_DIR, filename))
if npath.startswith(STATIC_DIR):
f = open(npath) # OK
f = open(npath) # $result=OK
@app.route("/path6")
@@ -62,7 +62,7 @@ def unsafe_path_abspath():
# Normalized (by `abspath`), but not checked properly
filename = request.args.get('filename', '')
npath = os.path.abspath(os.path.join(STATIC_DIR, filename))
f = open(npath) # NOT OK
f = open(npath) # $result=BAD
@app.route("/path7")
@@ -71,7 +71,7 @@ def safe_path_abspath():
filename = request.args.get('filename', '')
npath = os.path.abspath(os.path.join(STATIC_DIR, filename))
if npath.startswith(STATIC_DIR):
f = open(npath) # OK
f = open(npath) # $result=OK
@app.route("/abspath_tricky")
@@ -84,14 +84,14 @@ def safe_path_abspath_tricky():
filename = request.args.get('filename', '')
possibly_unsafe_path = os.path.join(STATIC_DIR, filename)
if os.path.abspath(possibly_unsafe_path).startswith(STATIC_DIR):
f = open(possibly_unsafe_path) # OK
f = open(possibly_unsafe_path) # $SPURIOUS: result=BAD
@app.route("/int-only/<int:foo_id>")
def flask_int_only(foo_id):
# This is OK, since the flask routing ensures that `foo_id` MUST be an integer.
path = os.path.join(STATIC_DIR, foo_id)
f = open(path) # OK TODO: FP
f = open(path) # $spurious: result=BAD
@app.route("/not-path/<foo>")
@@ -99,7 +99,7 @@ def flask_not_path(foo):
# On UNIX systems, this is OK, since without being marked as `<path:foo>`, flask
# routing ensures that `foo` cannot contain forward slashes (not by using %2F either).
path = os.path.join(STATIC_DIR, foo)
f = open(path) # OK if only running on UNIX systems, NOT OK if could be running on windows
f = open(path) # $result=BAD // OK if only running on UNIX systems, NOT OK if could be running on windows
@app.route("/no-dot-dot")
@@ -110,7 +110,7 @@ def no_dot_dot():
# handle if `filename` is an absolute path
if '../' in path:
return "not this time"
f = open(path) # NOT OK
f = open(path) # $result=BAD
@app.route("/no-dot-dot-with-prefix")
@@ -121,7 +121,7 @@ def no_dot_dot_with_prefix():
# Therefore, for UNIX-only programs, the `../` check is enough to stop path injections.
if '../' in path:
return "not this time"
f = open(path) # OK if only running on UNIX systems, NOT OK if could be running on windows
f = open(path) # $result=BAD // OK if only running on UNIX systems, NOT OK if could be running on windows
@app.route("/replace-slash")
@@ -129,7 +129,7 @@ def replace_slash():
filename = request.args.get('filename', '')
path = os.path.join(STATIC_DIR, filename)
sanitized = path.replace("/", "_")
f = open(sanitized) # OK if only running on UNIX systems, NOT OK if could be running on windows
f = open(sanitized) # $result=BAD // OK if only running on UNIX systems, NOT OK if could be running on windows
@app.route("/stackoverflow-solution")
@@ -139,7 +139,7 @@ def stackoverflow_solution():
path = os.path.join(STATIC_DIR, filename)
if os.path.commonprefix((os.path.realpath(path), STATIC_DIR)) != STATIC_DIR:
return "not this time"
f = open(path) # OK TODO: FP
f = open(path) # $SPURIOUS: result=BAD
SAFE_FILES = ['foo', 'bar', 'baz']
@@ -149,4 +149,4 @@ def safe_set_of_files():
filename = request.args.get('filename', '')
if filename in SAFE_FILES:
path = os.path.join(STATIC_DIR, filename)
f = open(path) # OK TODO: FP
f = open(path) # $SPURIOUS: result=BAD

10
python/ql/test/query-tests/Security/CWE-022-PathInjection/test.py
View File

@@ -16,21 +16,21 @@ def normalize(x):
@app.route("/path")
def simple():
x = source()
open(x) # NOT OK
open(x) # $result=BAD
@app.route("/path")
def normalization():
x = source()
y = normalize(x)
open(y) # NOT OK
open(y) # $result=BAD
@app.route("/path")
def check():
x = source()
if x.startswith("subfolder/"):
open(x) # NOT OK
open(x) # $result=BAD
@app.route("/path")
@@ -38,7 +38,7 @@ def normalize_then_check():
x = source()
y = normalize(x)
if y.startswith("subfolder/"):
open(y) # OK
open(y) # $result=OK
@app.route("/path")
@@ -46,4 +46,4 @@ def check_then_normalize():
x = source()
if x.startswith("subfolder/"):
y = normalize(x)
open(y) # NOT OK
open(y) # $result=BAD

6
python/ql/test/query-tests/Security/CWE-022-PathInjection/test_chaining.py
View File

@@ -20,7 +20,7 @@ def normalize_then_check():
x = source()
y = normalize(x) # <--- this call...
if y.startswith("subfolder/"):
open(y) # OK
open(y) # $result=OK
@app.route("/path")
@@ -29,7 +29,7 @@ def normalize_check_normalize():
y = normalize(x) # (...or this call...)
if y.startswith("subfolder/"):
z = normalize(y) # <--- ...can jump to here, resulting in FP
open(z) # OK
open(z) # $result=OK
# The problem does not manifest if we simply define an alias
@@ -42,4 +42,4 @@ def normalize_check_normalize_alias():
y = normpath(x)
if y.startswith("subfolder/"):
z = normpath(y)
open(z) # OK
open(z) # $result=OK

2
python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.expected Normal file
View File

@@ -0,0 +1,2 @@
missingAnnotationOnSink
failures

3
python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.ql Normal file
View File

@@ -0,0 +1,3 @@
import python
import experimental.dataflow.TestUtil.DataflowQueryTest
import semmle.python.security.dataflow.CommandInjectionQuery

26
python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py
View File

@@ -10,27 +10,27 @@ app = Flask(__name__)
def command_injection1():
files = request.args.get('files', '')
# Don't let files be `; rm -rf /`
os.system("ls " + files)
os.system("ls " + files) # $result=BAD
@app.route("/command2")
def command_injection2():
files = request.args.get('files', '')
# Don't let files be `; rm -rf /`
subprocess.Popen("ls " + files, shell=True)
subprocess.Popen("ls " + files, shell=True) # $result=BAD
@app.route("/command3")
def first_arg_injection():
cmd = request.args.get('cmd', '')
subprocess.Popen([cmd, "param1"])
subprocess.Popen([cmd, "param1"]) # $result=BAD
@app.route("/other_cases")
def others():
files = request.args.get('files', '')
# Don't let files be `; rm -rf /`
os.popen("ls " + files)
os.popen("ls " + files) # $result=BAD
@app.route("/multiple")
@@ -38,8 +38,8 @@ def multiple():
command = request.args.get('command', '')
# We should mark flow to both calls here, which conflicts with removing flow out of
# a sink due to use-use flow.
os.system(command)
os.system(command)
os.system(command) # $result=BAD
os.system(command) # $result=BAD
@app.route("/not-into-sink-impl")
@@ -52,11 +52,11 @@ def not_into_sink_impl():
subprocess.call implementation: https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
"""
command = request.args.get('command', '')
os.system(command)
os.popen(command)
subprocess.call(command)
subprocess.check_call(command)
subprocess.run(command)
os.system(command) # $result=BAD
os.popen(command) # $result=BAD
subprocess.call(command) # $result=BAD
subprocess.check_call(command) # $result=BAD
subprocess.run(command) # $result=BAD
@app.route("/path-exists-not-sanitizer")
@@ -70,11 +70,11 @@ def path_exists_not_sanitizer():
"""
path = request.args.get('path', '')
if os.path.exists(path):
os.system("ls " + path) # NOT OK
os.system("ls " + path) # $result=BAD
@app.route("/restricted-characters")
def restricted_characters():
path = request.args.get('path', '')
if re.match(r'^[a-zA-Z0-9_-]+$', path):
os.system("ls " + path) # OK (TODO: Currently FP)
os.system("ls " + path) # $SPURIOUS: result=BAD