Skip to content

Commit 56c85ff

Browse files
committed
Python: Fixup threat-models for os.environ.get()
Since using `.DictionaryElementAny` doesn't actually do a store on the source, (so we can later follow any dict read-steps). I added the ensure_tainted steps to highlight that the result of the WHOLE expression ends up "tainted", and that we don't just mark `os.environ` as the source without further flow.
1 parent b9239d7 commit 56c85ff

File tree

4 files changed

+33
-23
lines changed

4 files changed

+33
-23
lines changed

python/ql/lib/semmle/python/frameworks/Stdlib.model.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ extensions:
55
data:
66
- ['os', 'Member[getenv].ReturnValue', 'environment']
77
- ['os', 'Member[getenvb].ReturnValue', 'environment']
8-
- ['os', 'Member[environ].DictionaryElementAny', 'environment']
9-
- ['os', 'Member[environb].DictionaryElementAny', 'environment']
10-
- ['posix', 'Member[environ].DictionaryElementAny', 'environment']
8+
- ['os', 'Member[environ]', 'environment']
9+
- ['os', 'Member[environb]', 'environment']
10+
- ['posix', 'Member[environ]', 'environment']
1111

12-
- ['sys', 'Member[argv].DictionaryElementAny', 'commandargs']
13-
- ['sys', 'Member[orig_argv].DictionaryElementAny', 'commandargs']
12+
- ['sys', 'Member[argv]', 'commandargs']
13+
- ['sys', 'Member[orig_argv]', 'commandargs']
1414

1515
# TODO: argparse
1616
# TODO: input / read from stdin

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import semmle.python.dataflow.new.TaintTracking
1515
import semmle.python.dataflow.new.RemoteFlowSources
1616
import TestUtilities.InlineExpectationsTest
1717
private import semmle.python.dataflow.new.internal.PrintNode
18+
private import semmle.python.Concepts
1819

1920
DataFlow::Node shouldBeTainted() {
2021
exists(DataFlow::CallCfgNode call |
@@ -45,7 +46,7 @@ module Conf {
4546
source.(DataFlow::CfgNode).getNode() = call.getAnArg()
4647
)
4748
or
48-
source instanceof RemoteFlowSource
49+
source instanceof ThreatModelSource
4950
}
5051

5152
predicate isSink(DataFlow::Node sink) {

python/ql/test/library-tests/frameworks/stdlib/threat_models.py

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,25 @@
22
import sys
33
import posix
44

5-
os.getenv("foo") # $ threatModelSource[environment]=os.getenv(..)
6-
os.getenvb("bar") # $ threatModelSource[environment]=os.getenvb(..)
5+
ensure_tainted(
6+
os.getenv("foo"), # $ tainted threatModelSource[environment]=os.getenv(..)
7+
os.getenvb("bar"), # $ tainted threatModelSource[environment]=os.getenvb(..)
78

8-
os.environ["foo"] # $ threatModelSource[environment]=os.environ["foo"]
9-
os.environ.get("foo") # $ MISSING: threatModelSource[environment]=os.environ.get(..)
9+
os.environ["foo"], # $ tainted threatModelSource[environment]=os.environ
10+
os.environ.get("foo"), # $ tainted threatModelSource[environment]=os.environ
1011

11-
os.environb["bar"] # $ threatModelSource[environment]=os.environb["bar"]
12-
posix.environ[b"foo"] # $ threatModelSource[environment]=posix.environ[b"foo"]
12+
os.environb["bar"], # $ tainted threatModelSource[environment]=os.environb
13+
posix.environ[b"foo"], # $ tainted threatModelSource[environment]=posix.environ
1314

1415

15-
sys.argv[1] # $ threatModelSource[commandargs]=sys.argv[1]
16-
sys.orig_argv[1] # $ threatModelSource[commandargs]=sys.orig_argv[1]
16+
sys.argv[1], # $ tainted threatModelSource[commandargs]=sys.argv
17+
sys.orig_argv[1], # $ tainted threatModelSource[commandargs]=sys.orig_argv
18+
)
19+
20+
for k,v in os.environ.items(): # $ threatModelSource[environment]=os.environ
21+
ensure_tainted(k) # $ tainted
22+
ensure_tainted(v) # $ tainted
23+
1724

1825
########################################
1926
# argparse
@@ -23,21 +30,23 @@
2330
parser = argparse.ArgumentParser()
2431
parser.add_argument("foo")
2532

26-
args = parser.parse_args()
27-
args.foo # $ MISSING: threatModelSource[commandargs]=args.foo
33+
args = parser.parse_args() # $ MISSING: threatModelSource[commandargs]=parser.parse_args()
34+
ensure_tainted(args.foo) # $ MISSING: tainted
2835

29-
explicit_argv_parsing = parser.parse_args(sys.argv)
30-
explicit_argv_parsing.foo # $ MISSING: threatModelSource[commandargs]=explicit_argv_parsing.foo
36+
explicit_argv_parsing = parser.parse_args(sys.argv) # $ threatModelSource[commandargs]=sys.argv
37+
ensure_tainted(explicit_argv_parsing.foo) # $ MISSING: tainted
3138

3239
fake_args = parser.parse_args(["<foo>"])
33-
fake_args.foo
40+
ensure_not_tainted(fake_args.foo)
3441

3542
########################################
3643
# reading input from stdin
3744
########################################
3845

39-
sys.stdin.readline() # $ MISSING: threatModelSource
40-
input() # $ MISSING: threatModelSource
46+
ensure_tainted(
47+
sys.stdin.readline(), # $ MISSING: tainted threatModelSource
48+
input(), # $ MISSING: tainted threatModelSource
49+
)
4150

4251
########################################
4352
# socket
@@ -46,4 +55,4 @@
4655
import socket
4756
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
4857
s.connect(("example.com", 1234))
49-
s.recv(1024) # $ MISSING: threatModelSource[socket]
58+
ensure_tainted(s.recv(1024)) # $ MISSING: tainted threatModelSource[socket]

python/ql/test/library-tests/frameworks/stdlib/wsgiref_simple_server_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def func2(environ, start_response): # $ requestHandler
4545
start_response(status, headers) # $ headerWriteBulk=headers headerWriteBulkUnsanitized=name,value
4646
return [b"Hello"] # $ HttpResponse responseBody=List
4747

48-
case = sys.argv[1] # $ threatModelSource[commandargs]=sys.argv[1]
48+
case = sys.argv[1] # $ threatModelSource[commandargs]=sys.argv
4949
if case == "1":
5050
server = wsgiref.simple_server.WSGIServer(ADDRESS, wsgiref.simple_server.WSGIRequestHandler)
5151
server.set_app(func)

0 commit comments

Comments
 (0)