Skip to content

Commit

Permalink
New check: B113: TrojanSource - Bidirectional control characters (PyC…
Browse files Browse the repository at this point in the history
…QA#757)

* New check: B113: TrojanSource - Bidirectional control characters

* Handling Python source files using a non-UTF8 encoding

* Pleasing pep8

* Adding missing "SPDX-License-Identifier: Apache-2.0" comment

* Also forbidding \u200F

* Fixups for pre-commit hooks

* Fixing KeyError: 'file_data'

* Update issue.py

* Apply suggestions from code review

* Update bandit/plugins/trojansource.py

---------

Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>
Co-authored-by: Eric Brown <eric_wade_brown@yahoo.com>
  • Loading branch information
3 people authored Jun 24, 2024
1 parent 6142b7a commit ec384b8
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 4 deletions.
3 changes: 2 additions & 1 deletion bandit/core/issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Cwe:
MULTIPLE_BINDS = 605
IMPROPER_CHECK_OF_EXCEPT_COND = 703
INCORRECT_PERMISSION_ASSIGNMENT = 732
INAPPROPRIATE_ENCODING_FOR_OUTPUT_CONTEXT = 838

MITRE_URL_PATTERN = "https://cwe.mitre.org/data/definitions/%s.html"

Expand Down Expand Up @@ -84,7 +85,7 @@ def __init__(
ident=None,
lineno=None,
test_id="",
col_offset=0,
col_offset=-1,
end_col_offset=0,
):
self.severity = severity
Expand Down
10 changes: 10 additions & 0 deletions bandit/core/node_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,4 +286,14 @@ def process(self, data):
"""
f_ast = ast.parse(data)
self.generic_visit(f_ast)
# Run tests that do not require access to the AST,
# but only to the whole file source:
self.context = {
"file_data": self.fdata,
"filename": self.fname,
"lineno": 0,
"linerange": [0, 1],
"col_offset": 0,
}
self.update_scores(self.tester.run_tests(self.context, "File"))
return self.scores
6 changes: 5 additions & 1 deletion bandit/core/test_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ def checks(*args):
def wrapper(func):
if not hasattr(func, "_checks"):
func._checks = []
func._checks.extend(utils.check_ast_node(a) for a in args)
for arg in args:
if arg == "File":
func._checks.append("File")
else:
func._checks.append(utils.check_ast_node(arg))

LOG.debug("checks() decorator executed")
LOG.debug(" func._checks: %s", func._checks)
Expand Down
5 changes: 3 additions & 2 deletions bandit/core/tester.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def run_tests(self, raw_context, checktype):
tests = self.testset.get_tests(checktype)
for test in tests:
name = test.__name__
# execute test with the an instance of the context class
# execute test with an instance of the context class
temp_context = copy.copy(raw_context)
context = b_context.Context(temp_context)
try:
Expand All @@ -66,7 +66,8 @@ def run_tests(self, raw_context, checktype):
if result.lineno is None:
result.lineno = temp_context["lineno"]
result.linerange = temp_context["linerange"]
result.col_offset = temp_context["col_offset"]
if result.col_offset == -1:
result.col_offset = temp_context["col_offset"]
result.end_col_offset = temp_context.get(
"end_col_offset", 0
)
Expand Down
77 changes: 77 additions & 0 deletions bandit/plugins/trojansource.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#
# SPDX-License-Identifier: Apache-2.0
r"""
=====================================================
B613: TrojanSource - Bidirectional control characters
=====================================================
This plugin checks for the presence of unicode bidirectional control characters
in Python source files. Those characters can be embedded in comments and strings
to reorder source code characters in a way that changes its logic.
:Example:
.. code-block:: none
>> Issue: [B613:trojansource] A Python source file contains bidirectional control characters ('\u202e').
Severity: High Confidence: Medium
CWE: CWE-838 (https://cwe.mitre.org/data/definitions/838.html)
More Info: https://bandit.readthedocs.io/en/1.7.5/plugins/b113_trojansource.html
Location: examples/trojansource.py:4:25
3 access_level = "user"
4 if access_level != 'none‮⁦': # Check if admin ⁩⁦' and access_level != 'user
5 print("You are an admin.\n")
.. seealso::
.. [1] https://trojansource.codes/
.. [2] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-42574
.. versionadded:: 1.7.10
""" # noqa: E501
from tokenize import detect_encoding

import bandit
from bandit.core import issue
from bandit.core import test_properties as test


BIDI_CHARACTERS = (
"\u202A",
"\u202B",
"\u202C",
"\u202D",
"\u202E",
"\u2066",
"\u2067",
"\u2068",
"\u2069",
"\u200F",
)


@test.test_id("B613")
@test.checks("File")
def trojansource(context):
with open(context.filename, "rb") as src_file:
encoding, _ = detect_encoding(src_file.readline)
with open(context.filename, encoding=encoding) as src_file:
for lineno, line in enumerate(src_file.readlines(), start=1):
for char in BIDI_CHARACTERS:
try:
col_offset = line.index(char) + 1
except ValueError:
continue
text = (
"A Python source file contains bidirectional"
" control characters (%r)." % char
)
return bandit.Issue(
severity=bandit.HIGH,
confidence=bandit.MEDIUM,
cwe=issue.Cwe.INAPPROPRIATE_ENCODING_FOR_OUTPUT_CONTEXT,
text=text,
lineno=lineno,
col_offset=col_offset,
)
5 changes: 5 additions & 0 deletions doc/source/plugins/trojansource.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
------------------
B613: trojansource
------------------

.. automodule:: bandit.plugins.trojansource
5 changes: 5 additions & 0 deletions examples/trojansource.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env python3
# cf. https://trojansource.codes/ & https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-42574
access_level = "user"
if access_level != 'none‮⁦': # Check if admin ⁩⁦' and access_level != 'user
print("You are an admin.\n")
7 changes: 7 additions & 0 deletions examples/trojansource_latin1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env python3
# -*- coding: latin-1 -*-
# cf. https://trojansource.codes & https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-42574
# Some special characters: àçéêèù
access_level = "user"
if access_level != 'none??': # Check if admin ??' and access_level != 'user
print("You are an admin.\n")
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ bandit.plugins =
#bandit/plugins/tarfile_unsafe_members.py
tarfile_unsafe_members = bandit.plugins.tarfile_unsafe_members:tarfile_unsafe_members

# bandit/plugins/trojansource.py
trojansource = bandit.plugins.trojansource:trojansource

[build_sphinx]
all_files = 1
build-dir = doc/build
Expand Down
14 changes: 14 additions & 0 deletions tests/functional/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -929,3 +929,17 @@ def test_tarfile_unsafe_members(self):
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 1, "MEDIUM": 2, "HIGH": 2},
}
self.check_example("tarfile_extractall.py", expect)

def test_trojansource(self):
expect = {
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 1},
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 1, "HIGH": 0},
}
self.check_example("trojansource.py", expect)

def test_trojansource_latin1(self):
expect = {
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 0},
}
self.check_example("trojansource_latin1.py", expect)

0 comments on commit ec384b8

Please sign in to comment.