Skip to content

Commit 4fd0ada

Browse files
yoffyoff
authored
Merge pull request github#7652 from RasmusWL/cleartext-remove-fps
Python: Remove usernames as sensitive source for cleartext queries
2 parents b02f1c8 + f53dce3 commit 4fd0ada

File tree

7 files changed

+72
-0
lines changed

7 files changed

+72
-0
lines changed

‎python/ql/lib/semmle/python/security/dataflow/CleartextLoggingCustomizations.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ module CleartextLogging {
4040
* A source of sensitive data, considered as a flow source.
4141
*/
4242
class SensitiveDataSourceAsSource extends Source, SensitiveDataSource {
43+
SensitiveDataSourceAsSource() {
44+
not SensitiveDataSource.super.getClassification() = SensitiveDataClassification::id()
45+
}
46+
4347
override SensitiveDataClassification getClassification() {
4448
result = SensitiveDataSource.super.getClassification()
4549
}

‎python/ql/lib/semmle/python/security/dataflow/CleartextStorageCustomizations.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ module CleartextStorage {
3939
* A source of sensitive data, considered as a flow source.
4040
*/
4141
class SensitiveDataSourceAsSource extends Source, SensitiveDataSource {
42+
SensitiveDataSourceAsSource() {
43+
not SensitiveDataSource.super.getClassification() = SensitiveDataClassification::id()
44+
}
45+
4246
override SensitiveDataClassification getClassification() {
4347
result = SensitiveDataSource.super.getClassification()
4448
}

‎python/ql/src/change-notes/2021-01-19-remove-cleartext-fps.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* User names and other account information is no longer considered to be sensitive data for the queries `py/clear-text-logging-sensitive-data` and `py/clear-text-storage-sensitive-data`, since this lead to many false positives.

‎python/ql/test/experimental/dataflow/sensitive-data/test.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,16 @@ def call_wrapper(func):
112112
harmless = lambda: "bar"
113113
bar = call_wrapper(harmless)
114114
print(bar) # $ SPURIOUS: SensitiveUse=password
115+
116+
# ------------------------------------------------------------------------------
117+
# cross-talk in dictionary.
118+
# ------------------------------------------------------------------------------
119+
120+
from unknown_settings import password # $ SensitiveDataSource=password
121+
122+
print(password) # $ SensitiveUse=password
123+
_config = {"sleep_timer": 5, "mysql_password": password}
124+
125+
# since we have taint-step from store of `password`, we will consider any item in the
126+
# dictionary to be a password :(
127+
print(_config["sleep_timer"]) # $ SPURIOUS: SensitiveUse=password

‎python/ql/test/query-tests/Security/CWE-312-CleartextLogging/CleartextLogging.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ edges
44
| test.py:19:16:19:29 | ControlFlowNode for get_password() | test.py:23:58:23:65 | ControlFlowNode for password |
55
| test.py:19:16:19:29 | ControlFlowNode for get_password() | test.py:27:40:27:47 | ControlFlowNode for password |
66
| test.py:19:16:19:29 | ControlFlowNode for get_password() | test.py:30:58:30:65 | ControlFlowNode for password |
7+
| test.py:65:14:68:5 | ControlFlowNode for Dict | test.py:69:11:69:31 | ControlFlowNode for Subscript |
8+
| test.py:67:21:67:37 | ControlFlowNode for Attribute | test.py:65:14:68:5 | ControlFlowNode for Dict |
79
nodes
810
| test.py:19:16:19:29 | ControlFlowNode for get_password() | semmle.label | ControlFlowNode for get_password() |
911
| test.py:20:48:20:55 | ControlFlowNode for password | semmle.label | ControlFlowNode for password |
@@ -15,6 +17,9 @@ nodes
1517
| test.py:37:11:37:24 | ControlFlowNode for get_password() | semmle.label | ControlFlowNode for get_password() |
1618
| test.py:39:22:39:35 | ControlFlowNode for get_password() | semmle.label | ControlFlowNode for get_password() |
1719
| test.py:40:22:40:35 | ControlFlowNode for get_password() | semmle.label | ControlFlowNode for get_password() |
20+
| test.py:65:14:68:5 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
21+
| test.py:67:21:67:37 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
22+
| test.py:69:11:69:31 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
1823
subpaths
1924
#select
2025
| test.py:20:48:20:55 | ControlFlowNode for password | test.py:19:16:19:29 | ControlFlowNode for get_password() | test.py:20:48:20:55 | ControlFlowNode for password | $@ is logged here. | test.py:19:16:19:29 | ControlFlowNode for get_password() | Sensitive data (password) |
@@ -26,3 +31,4 @@ subpaths
2631
| test.py:37:11:37:24 | ControlFlowNode for get_password() | test.py:37:11:37:24 | ControlFlowNode for get_password() | test.py:37:11:37:24 | ControlFlowNode for get_password() | $@ is logged here. | test.py:37:11:37:24 | ControlFlowNode for get_password() | Sensitive data (password) |
2732
| test.py:39:22:39:35 | ControlFlowNode for get_password() | test.py:39:22:39:35 | ControlFlowNode for get_password() | test.py:39:22:39:35 | ControlFlowNode for get_password() | $@ is logged here. | test.py:39:22:39:35 | ControlFlowNode for get_password() | Sensitive data (password) |
2833
| test.py:40:22:40:35 | ControlFlowNode for get_password() | test.py:40:22:40:35 | ControlFlowNode for get_password() | test.py:40:22:40:35 | ControlFlowNode for get_password() | $@ is logged here. | test.py:40:22:40:35 | ControlFlowNode for get_password() | Sensitive data (password) |
34+
| test.py:69:11:69:31 | ControlFlowNode for Subscript | test.py:67:21:67:37 | ControlFlowNode for Attribute | test.py:69:11:69:31 | ControlFlowNode for Subscript | $@ is logged here. | test.py:67:21:67:37 | ControlFlowNode for Attribute | Sensitive data (password) |

‎python/ql/test/query-tests/Security/CWE-312-CleartextLogging/test.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,36 @@ def print_password():
3939
sys.stdout.write(get_password()) # NOT OK
4040
sys.stderr.write(get_password()) # NOT OK
4141

42+
43+
def FPs(account, account_id):
44+
# we assume that any account parameter is sensitive (id/username)
45+
# https://github.com/github/codeql/issues/6363
46+
print(account) # OK
47+
48+
# https://github.com/github/codeql/issues/6927
49+
arn = f"arn:aws:iam::{account_id}:role/cfripper-access"
50+
logging.info(f"Preparing to assume role: {arn}") # OK
51+
52+
# Harmless UUIDs
53+
# https://github.com/github/codeql/issues/6726
54+
# https://github.com/github/codeql/issues/7497
55+
x = generate_uuid4()
56+
print(x) # OK
57+
58+
# username not considered sensitive
59+
# https://github.com/github/codeql/issues/7116
60+
logging.error("Misc Exception. User %s: %s", request.user.username)
61+
62+
# dictionary taint-flow cross-talk
63+
# https://github.com/github/codeql/issues/6380
64+
import settings
65+
config = {
66+
"sleep_timer": 5,
67+
"password": settings.password
68+
}
69+
print(config["sleep_timer"]) # OK
70+
71+
4272
if __name__ == "__main__":
4373
logging.basicConfig(level=logging.DEBUG)
4474
log_password()

‎python/ql/test/query-tests/Security/CWE-312-CleartextStorage/test.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,14 @@ def write_cert(filename):
88
file.write(cert) # NOT OK
99
lines = [cert + "\n"]
1010
file.writelines(lines) # NOT OK
11+
12+
def FPs():
13+
# just like for cleartext-logging see that file for more elaborate tests
14+
#
15+
# this part is just to make sure the two queries are in line with what is considered
16+
# sensitive information.
17+
18+
with open(filename, "w") as file:
19+
# Harmless UUIDs
20+
x = generate_uuid4()
21+
file.write(x) # OK

0 commit comments

Comments
 (0)