From ec384b84261e563d4dc5cfe71673f159fcb199af Mon Sep 17 00:00:00 2001 From: Lucas Cimon <925560+Lucas-C@users.noreply.github.com> Date: Mon, 24 Jun 2024 05:53:33 +0200 Subject: [PATCH] New check: B113: TrojanSource - Bidirectional control characters (#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 Co-authored-by: Eric Brown --- bandit/core/issue.py | 3 +- bandit/core/node_visitor.py | 10 ++++ bandit/core/test_properties.py | 6 ++- bandit/core/tester.py | 5 +- bandit/plugins/trojansource.py | 77 +++++++++++++++++++++++++++++ doc/source/plugins/trojansource.rst | 5 ++ examples/trojansource.py | 5 ++ examples/trojansource_latin1.py | 7 +++ setup.cfg | 3 ++ tests/functional/test_functional.py | 14 ++++++ 10 files changed, 131 insertions(+), 4 deletions(-) create mode 100755 bandit/plugins/trojansource.py create mode 100644 doc/source/plugins/trojansource.rst create mode 100644 examples/trojansource.py create mode 100644 examples/trojansource_latin1.py diff --git a/bandit/core/issue.py b/bandit/core/issue.py index 875e5e418..bfa583356 100644 --- a/bandit/core/issue.py +++ b/bandit/core/issue.py @@ -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" @@ -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 diff --git a/bandit/core/node_visitor.py b/bandit/core/node_visitor.py index 26cdb2471..27a4de5ee 100644 --- a/bandit/core/node_visitor.py +++ b/bandit/core/node_visitor.py @@ -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 diff --git a/bandit/core/test_properties.py b/bandit/core/test_properties.py index cf969952f..f6d4da1a7 100644 --- a/bandit/core/test_properties.py +++ b/bandit/core/test_properties.py @@ -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) diff --git a/bandit/core/tester.py b/bandit/core/tester.py index af5ffdae9..6d41877cb 100644 --- a/bandit/core/tester.py +++ b/bandit/core/tester.py @@ -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: @@ -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 ) diff --git a/bandit/plugins/trojansource.py b/bandit/plugins/trojansource.py new file mode 100755 index 000000000..5c0eae5eb --- /dev/null +++ b/bandit/plugins/trojansource.py @@ -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, + ) diff --git a/doc/source/plugins/trojansource.rst b/doc/source/plugins/trojansource.rst new file mode 100644 index 000000000..8fa0bc47b --- /dev/null +++ b/doc/source/plugins/trojansource.rst @@ -0,0 +1,5 @@ +------------------ +B613: trojansource +------------------ + +.. automodule:: bandit.plugins.trojansource diff --git a/examples/trojansource.py b/examples/trojansource.py new file mode 100644 index 000000000..40c605579 --- /dev/null +++ b/examples/trojansource.py @@ -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") diff --git a/examples/trojansource_latin1.py b/examples/trojansource_latin1.py new file mode 100644 index 000000000..dee24e07c --- /dev/null +++ b/examples/trojansource_latin1.py @@ -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") diff --git a/setup.cfg b/setup.cfg index 2dbee597c..52128b17d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -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 diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py index a92fe3f9c..4597f7023 100644 --- a/tests/functional/test_functional.py +++ b/tests/functional/test_functional.py @@ -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)