Skip to content

Commit 716627c

Browse files
yoffyoff
authored
Merge pull request github#5878 from RasmusWL/fix-concept-tests-pretty-print
Python: Fix concept tests pretty print
2 parents b14fa8b + 61ad5d0 commit 716627c

File tree

17 files changed

+150
-151
lines changed

17 files changed

+150
-151
lines changed

‎python/ql/test/experimental/dataflow/TestUtil/PrintNode.qll

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import python
22
import semmle.python.dataflow.new.DataFlow
33

4-
string prettyExp(Expr e) {
4+
string prettyExpr(Expr e) {
55
not e instanceof Num and
66
not e instanceof StrConst and
77
not e instanceof Subscript and
@@ -15,17 +15,41 @@ string prettyExp(Expr e) {
1515
e.(StrConst).getPrefix() + e.(StrConst).getText() +
1616
e.(StrConst).getPrefix().regexpReplaceAll("[a-zA-Z]+", "")
1717
or
18-
result = prettyExp(e.(Subscript).getObject()) + "[" + prettyExp(e.(Subscript).getIndex()) + "]"
18+
result = prettyExpr(e.(Subscript).getObject()) + "[" + prettyExpr(e.(Subscript).getIndex()) + "]"
1919
or
2020
(
2121
if exists(e.(Call).getAnArg()) or exists(e.(Call).getANamedArg())
22-
then result = prettyExp(e.(Call).getFunc()) + "(..)"
23-
else result = prettyExp(e.(Call).getFunc()) + "()"
22+
then result = prettyExpr(e.(Call).getFunc()) + "(..)"
23+
else result = prettyExpr(e.(Call).getFunc()) + "()"
2424
)
2525
or
26-
result = prettyExp(e.(Attribute).getObject()) + "." + e.(Attribute).getName()
26+
result = prettyExpr(e.(Attribute).getObject()) + "." + e.(Attribute).getName()
2727
}
2828

29+
/**
30+
* Gets pretty-printed version of the DataFlow::Node `node`
31+
*/
32+
bindingset[node]
2933
string prettyNode(DataFlow::Node node) {
30-
if exists(node.asExpr()) then result = prettyExp(node.asExpr()) else result = node.toString()
34+
if exists(node.asExpr()) then result = prettyExpr(node.asExpr()) else result = node.toString()
35+
}
36+
37+
/**
38+
* Gets pretty-printed version of the DataFlow::Node `node`, that is suitable for use
39+
* with `TestUtilities.InlineExpectationsTest` (that is, no spaces unless required).
40+
*/
41+
bindingset[node]
42+
string prettyNodeForInlineTest(DataFlow::Node node) {
43+
exists(node.asExpr()) and
44+
result = prettyExpr(node.asExpr())
45+
or
46+
exists(Expr e | e = node.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() |
47+
// since PostUpdateNode both has space in the `[post <thing>]` annotation, and does
48+
// not pretty print the pre-update node, we do custom handling of this.
49+
result = "[post]" + prettyExpr(e)
50+
)
51+
or
52+
not exists(node.asExpr()) and
53+
not exists(Expr e | e = node.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr()) and
54+
result = node.toString()
3155
}

‎python/ql/test/experimental/dataflow/tainttracking/TestTaintLib.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,6 @@ query predicate test_taint(string arg_location, string test_res, string scope_na
4646
arg_location = arg.getLocation().toString() and
4747
test_res = test_res and
4848
scope_name = call.getScope().getName() and
49-
repr = prettyExp(arg)
49+
repr = prettyExpr(arg)
5050
)
5151
}

‎python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/test_json.py

Lines changed: 0 additions & 53 deletions
This file was deleted.

‎python/ql/test/experimental/meta/ConceptsTest.qll

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,7 @@ import python
22
import semmle.python.dataflow.new.DataFlow
33
import semmle.python.Concepts
44
import TestUtilities.InlineExpectationsTest
5-
6-
string value_from_expr(Expr e) {
7-
// TODO: This one is starting to look like `repr` predicate from TestTaintLib
8-
result =
9-
e.(StrConst).getPrefix() + e.(StrConst).getText() +
10-
e.(StrConst).getPrefix().regexpReplaceAll("[a-zA-Z]+", "")
11-
or
12-
result = e.(Name).getId()
13-
or
14-
not e instanceof StrConst and
15-
not e instanceof Name and
16-
result = e.toString()
17-
}
5+
import experimental.dataflow.TestUtil.PrintNode
186

197
class SystemCommandExecutionTest extends InlineExpectationsTest {
208
SystemCommandExecutionTest() { this = "SystemCommandExecutionTest" }
@@ -27,7 +15,7 @@ class SystemCommandExecutionTest extends InlineExpectationsTest {
2715
command = sce.getCommand() and
2816
location = command.getLocation() and
2917
element = command.toString() and
30-
value = value_from_expr(command.asExpr()) and
18+
value = prettyNodeForInlineTest(command) and
3119
tag = "getCommand"
3220
)
3321
}
@@ -46,7 +34,7 @@ class DecodingTest extends InlineExpectationsTest {
4634
exists(DataFlow::Node data |
4735
location = data.getLocation() and
4836
element = data.toString() and
49-
value = value_from_expr(data.asExpr()) and
37+
value = prettyNodeForInlineTest(data) and
5038
(
5139
data = d.getAnInput() and
5240
tag = "decodeInput"
@@ -84,7 +72,7 @@ class EncodingTest extends InlineExpectationsTest {
8472
exists(DataFlow::Node data |
8573
location = data.getLocation() and
8674
element = data.toString() and
87-
value = value_from_expr(data.asExpr()) and
75+
value = prettyNodeForInlineTest(data) and
8876
(
8977
data = e.getAnInput() and
9078
tag = "encodeInput"
@@ -117,7 +105,7 @@ class CodeExecutionTest extends InlineExpectationsTest {
117105
code = ce.getCode() and
118106
location = code.getLocation() and
119107
element = code.toString() and
120-
value = value_from_expr(code.asExpr()) and
108+
value = prettyNodeForInlineTest(code) and
121109
tag = "getCode"
122110
)
123111
}
@@ -135,7 +123,7 @@ class SqlExecutionTest extends InlineExpectationsTest {
135123
sql = e.getSql() and
136124
location = e.getLocation() and
137125
element = sql.toString() and
138-
value = value_from_expr(sql.asExpr()) and
126+
value = prettyNodeForInlineTest(sql) and
139127
tag = "getSql"
140128
)
141129
}
@@ -218,7 +206,7 @@ class HttpServerHttpResponseTest extends InlineExpectationsTest {
218206
exists(HTTP::Server::HttpResponse response |
219207
location = response.getLocation() and
220208
element = response.toString() and
221-
value = value_from_expr(response.getBody().asExpr()) and
209+
value = prettyNodeForInlineTest(response.getBody()) and
222210
tag = "responseBody"
223211
)
224212
or
@@ -257,7 +245,7 @@ class HttpServerHttpRedirectResponseTest extends InlineExpectationsTest {
257245
exists(HTTP::Server::HttpRedirectResponse redirect |
258246
location = redirect.getLocation() and
259247
element = redirect.toString() and
260-
value = value_from_expr(redirect.getRedirectLocation().asExpr()) and
248+
value = prettyNodeForInlineTest(redirect.getRedirectLocation()) and
261249
tag = "redirectLocation"
262250
)
263251
)
@@ -275,7 +263,7 @@ class FileSystemAccessTest extends InlineExpectationsTest {
275263
path = a.getAPathArgument() and
276264
location = a.getLocation() and
277265
element = path.toString() and
278-
value = value_from_expr(path.asExpr()) and
266+
value = prettyNodeForInlineTest(path) and
279267
tag = "getAPathArgument"
280268
)
281269
}
@@ -309,7 +297,7 @@ class SafeAccessCheckTest extends InlineExpectationsTest {
309297
location = c.getLocation() and
310298
(
311299
element = checks.toString() and
312-
value = value_from_expr(checks.asExpr()) and
300+
value = prettyNodeForInlineTest(checks) and
313301
tag = "checks"
314302
or
315303
element = branch.toString() and

‎python/ql/test/experimental/meta/InlineTaintTest.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class InlineTaintTest extends InlineExpectationsTest {
5858
exists(DataFlow::Node sink |
5959
any(TestTaintTrackingConfiguration config).hasFlow(_, sink) and
6060
location = sink.getLocation() and
61-
element = prettyExp(sink.asExpr()) and
61+
element = prettyExpr(sink.asExpr()) and
6262
value = "" and
6363
tag = "tainted"
6464
)
@@ -84,7 +84,7 @@ query predicate untaintedArgumentToEnsureTaintedNotMarkedAsMissing(
8484
error = "ERROR, you should add `# $ MISSING: tainted` annotation" and
8585
exists(DataFlow::Node sink |
8686
sink = shouldBeTainted() and
87-
element = prettyExp(sink.asExpr()) and
87+
element = prettyExpr(sink.asExpr()) and
8888
not any(TestTaintTrackingConfiguration config).hasFlow(_, sink) and
8989
location = sink.getLocation() and
9090
not exists(FalseNegativeExpectation missingResult |

‎python/ql/test/library-tests/frameworks/dill/Decoding.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
import dill
22

3-
dill.loads(payload) # $decodeInput=payload decodeOutput=Attribute() decodeFormat=dill decodeMayExecuteInput
3+
dill.loads(payload) # $decodeInput=payload decodeOutput=dill.loads(..) decodeFormat=dill decodeMayExecuteInput

‎python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,28 +74,28 @@ def get_redirect_url(self, foo): # $ requestHandler routedParameter=foo
7474

7575
# Ensure that simple subclasses are still vuln to XSS
7676
def xss__not_found(request):
77-
return HttpResponseNotFound(request.GET.get("name")) # $HttpResponse mimetype=text/html responseBody=Attribute()
77+
return HttpResponseNotFound(request.GET.get("name")) # $HttpResponse mimetype=text/html responseBody=request.GET.get(..)
7878

7979
# Ensure we still have an XSS sink when manually setting the content_type to HTML
8080
def xss__manual_response_type(request):
81-
return HttpResponse(request.GET.get("name"), content_type="text/html; charset=utf-8") # $HttpResponse mimetype=text/html responseBody=Attribute()
81+
return HttpResponse(request.GET.get("name"), content_type="text/html; charset=utf-8") # $HttpResponse mimetype=text/html responseBody=request.GET.get(..)
8282

8383
def xss__write(request):
8484
response = HttpResponse() # $HttpResponse mimetype=text/html
85-
response.write(request.GET.get("name")) # $HttpResponse mimetype=text/html responseBody=Attribute()
85+
response.write(request.GET.get("name")) # $HttpResponse mimetype=text/html responseBody=request.GET.get(..)
8686

8787
# This is safe but probably a bug if the argument to `write` is not a result of `json.dumps` or similar.
8888
def safe__write_json(request):
8989
response = JsonResponse() # $HttpResponse mimetype=application/json
90-
response.write(request.GET.get("name")) # $HttpResponse mimetype=application/json responseBody=Attribute()
90+
response.write(request.GET.get("name")) # $HttpResponse mimetype=application/json responseBody=request.GET.get(..)
9191

9292
# Ensure manual subclasses are vulnerable
9393
class CustomResponse(HttpResponse):
9494
def __init__(self, banner, content, *args, **kwargs):
9595
super().__init__(content, *args, content_type="text/html", **kwargs)
9696

9797
def xss__custom_response(request):
98-
return CustomResponse("ACME Responses", request.GET("name")) # $HttpResponse MISSING: mimetype=text/html responseBody=Attribute() SPURIOUS: responseBody="ACME Responses"
98+
return CustomResponse("ACME Responses", request.GET("name")) # $HttpResponse MISSING: mimetype=text/html responseBody=request.GET.get(..) SPURIOUS: responseBody="ACME Responses"
9999

100100
class CustomJsonResponse(JsonResponse):
101101
def __init__(self, banner, content, *args, **kwargs):

‎python/ql/test/library-tests/frameworks/django-v2-v3/testproj/settings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from pathlib import Path
1414

1515
# Build paths inside the project like this: BASE_DIR / 'subdir'.
16-
BASE_DIR = Path(__file__).resolve().parent.parent #$ getAPathArgument=Path()
16+
BASE_DIR = Path(__file__).resolve().parent.parent #$ getAPathArgument=Path(..)
1717

1818

1919
# Quick-start development settings - unsuitable for production

‎python/ql/test/library-tests/frameworks/idna/taint_test.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ def test_idna():
55
tb = TAINTED_BYTES
66

77
ensure_tainted(
8-
idna.encode(ts), # $ tainted encodeInput=ts encodeOutput=Attribute() encodeFormat=IDNA
9-
idna.encode(s=ts), # $ tainted encodeInput=ts encodeOutput=Attribute() encodeFormat=IDNA
8+
idna.encode(ts), # $ tainted encodeInput=ts encodeOutput=idna.encode(..) encodeFormat=IDNA
9+
idna.encode(s=ts), # $ tainted encodeInput=ts encodeOutput=idna.encode(..) encodeFormat=IDNA
1010

11-
idna.decode(tb), # $ tainted decodeInput=tb decodeOutput=Attribute() decodeFormat=IDNA
12-
idna.decode(s=tb), # $ tainted decodeInput=tb decodeOutput=Attribute() decodeFormat=IDNA
11+
idna.decode(tb), # $ tainted decodeInput=tb decodeOutput=idna.decode(..) decodeFormat=IDNA
12+
idna.decode(s=tb), # $ tainted decodeInput=tb decodeOutput=idna.decode(..) decodeFormat=IDNA
1313
)

‎python/ql/test/library-tests/frameworks/simplejson/taint_test.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ def test():
55
ts = TAINTED_STRING
66
tainted_obj = {"foo": ts}
77

8-
encoded = simplejson.dumps(tainted_obj) # $ encodeOutput=Attribute() encodeFormat=JSON encodeInput=tainted_obj
8+
encoded = simplejson.dumps(tainted_obj) # $ encodeOutput=simplejson.dumps(..) encodeFormat=JSON encodeInput=tainted_obj
99

1010
ensure_tainted(
1111
encoded, # $ tainted
12-
simplejson.dumps(tainted_obj), # $ tainted encodeOutput=Attribute() encodeFormat=JSON encodeInput=tainted_obj
13-
simplejson.dumps(obj=tainted_obj), # $ tainted encodeOutput=Attribute() encodeFormat=JSON encodeInput=tainted_obj
14-
simplejson.loads(encoded), # $ tainted decodeOutput=Attribute() decodeFormat=JSON decodeInput=encoded
15-
simplejson.loads(s=encoded), # $ tainted decodeOutput=Attribute() decodeFormat=JSON decodeInput=encoded
12+
simplejson.dumps(tainted_obj), # $ tainted encodeOutput=simplejson.dumps(..) encodeFormat=JSON encodeInput=tainted_obj
13+
simplejson.dumps(obj=tainted_obj), # $ tainted encodeOutput=simplejson.dumps(..) encodeFormat=JSON encodeInput=tainted_obj
14+
simplejson.loads(encoded), # $ tainted decodeOutput=simplejson.loads(..) decodeFormat=JSON decodeInput=encoded
15+
simplejson.loads(s=encoded), # $ tainted decodeOutput=simplejson.loads(..) decodeFormat=JSON decodeInput=encoded
1616
)
1717

1818
# load/dump with file-like
@@ -22,7 +22,7 @@ def test():
2222
tainted_filelike.seek(0)
2323
ensure_tainted(
2424
tainted_filelike, # $ MISSING: tainted
25-
simplejson.load(tainted_filelike), # $ decodeOutput=Attribute() decodeFormat=JSON decodeInput=tainted_filelike MISSING: tainted
25+
simplejson.load(tainted_filelike), # $ decodeOutput=simplejson.load(..) decodeFormat=JSON decodeInput=tainted_filelike MISSING: tainted
2626
)
2727

2828
# load/dump with file-like using keyword-args
@@ -32,7 +32,7 @@ def test():
3232
tainted_filelike.seek(0)
3333
ensure_tainted(
3434
tainted_filelike, # $ MISSING: tainted
35-
simplejson.load(fp=tainted_filelike), # $ decodeOutput=Attribute() decodeFormat=JSON decodeInput=tainted_filelike MISSING: tainted
35+
simplejson.load(fp=tainted_filelike), # $ decodeOutput=simplejson.load(..) decodeFormat=JSON decodeInput=tainted_filelike MISSING: tainted
3636
)
3737

3838
# To make things runable

0 commit comments

Comments
 (0)