From f2db5824c692a590639770c3cde2855bcfeb534a Mon Sep 17 00:00:00 2001 From: Eric Brown Date: Wed, 17 Jul 2024 13:53:05 -0700 Subject: [PATCH] Add new rule around loose permission in os module Any usage of os.chmod and the like, the mode for the permissions for a file should be checked to ensure the permissions are not considered loose or incorrect. Incorrect in this case is using world write, group write, world execute, group execute. Care should be taken when using those. Signed-off-by: Eric Brown --- .../rules/python/stdlib/os_loose_file_perm.py | 179 ++++++++++++++++++ setup.cfg | 3 + .../python/stdlib/os/examples/os_chmod_511.py | 9 + .../stdlib/os/examples/os_chmod_o227.py | 10 + .../stdlib/os/examples/os_chmod_o644.py | 9 + .../python/stdlib/os/examples/os_chmod_o7.py | 9 + .../os/examples/os_chmod_o755_binop_stat.py | 14 ++ .../stdlib/os/examples/os_chmod_o760.py | 9 + .../stdlib/os/examples/os_chmod_o770.py | 9 + .../stdlib/os/examples/os_chmod_o776.py | 9 + .../stdlib/os/examples/os_chmod_o777.py | 11 ++ .../os/examples/os_chmod_stat_S_IXOTH.py | 10 + .../stdlib/os/examples/os_chmod_x1ff.py | 11 ++ .../stdlib/os/test_os_loose_file_perm.py | 57 ++++++ 14 files changed, 349 insertions(+) create mode 100644 precli/rules/python/stdlib/os_loose_file_perm.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_511.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o227.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o644.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o7.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o755_binop_stat.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o760.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o770.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o776.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_o777.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_stat_S_IXOTH.py create mode 100644 tests/unit/rules/python/stdlib/os/examples/os_chmod_x1ff.py create mode 100644 tests/unit/rules/python/stdlib/os/test_os_loose_file_perm.py diff --git a/precli/rules/python/stdlib/os_loose_file_perm.py b/precli/rules/python/stdlib/os_loose_file_perm.py new file mode 100644 index 00000000..f93df66d --- /dev/null +++ b/precli/rules/python/stdlib/os_loose_file_perm.py @@ -0,0 +1,179 @@ +# Copyright 2024 Secure Sauce LLC +r""" +# Incorrect Permission Assignment for Critical Resource using `os` Module + +The Python module `crypt` provides a number of functions for password +hashing. However, some of the hashing functions supported by `crypt` are weak +and should not be used. These weak hashing functions include `CRYPT` and +`MD5`. + +The `CRYPT` hashing function is a weak hashing function because it is based +on a simple DES algorithm. This algorithm is relatively easy to crack, and +passwords hashed with crypt can be easily recovered by attackers. + +The `MD5` hashing function is also a weak hashing function. MD5 is a +cryptographic hash function that was designed in the early 1990s. MD5 is +no longer considered secure, and passwords hashed with MD5 can be easily +cracked by attackers. + +If using the crypt module, it is recommended to use more secure methods such +as `SHA256` and `SHA512`. + +## Examples + +```python +import crypt + + +crypt.crypt("password", salt=crypt.METHOD_MD5) +``` + +```python +import crypt + + +crypt.mksalt(crypt.METHOD_CRYPT) +``` + +## Remediation + +The recommendation is to swap the insecure hashing method to one of the more +secure alternatives, `SHA256` or `SHA512`. + +```python +import crypt + + +crypt.crypt("password", salt=crypt.METHOD_SHA256) +``` + +```python +import crypt + + +crypt.mksalt(crypt.METHOD_SHA512) +``` + +## Alternatives to Crypt + +There are a number of alternatives to weak hashing functions. These +alternatives include `bcrypt`, `pbkdf2`, and `scrypt`. + + - `bcrypt` is a secure password hashing function that is based on the + Blowfish block cipher. Bcrypt is considered to be one of the most secure + password hashing functions available. + + - `PBKDF2` is a secure password hashing function that is based on the HMAC + cryptographic function. PBKDF2 is considered to be one of the most secure + password hashing functions available. + + - `scrypt` is a secure password hashing function that is based on the bcrypt + algorithm. Scrypt is designed to be more secure than bcrypt, and it is also + more resistant to GPU-based attacks. + +## See also + +- [crypt — Function to check Unix passwords](https://docs.python.org/3/library/crypt.html) +- [CWE-732: Incorrect Permission Assignment for Critical Resource](https://cwe.mitre.org/data/definitions/732.html) + +_New in version 0.6.2_ + +""" # noqa: E501 +import stat + +from precli.core.call import Call +from precli.core.location import Location +from precli.core.result import Result +from precli.parsers import tokens +from precli.rules import Rule + + +def _stat_is_dangerous(mode): + return ( + mode & stat.S_IWOTH + or mode & stat.S_IWGRP + or mode & stat.S_IXGRP + or mode & stat.S_IXOTH + ) + + +def resolve_binop(node): + value = 0 + + for child in node.named_children: + print(child.string) + + if child.type == tokens.BINARY_OPERATOR: + value |= resolve_binop(child) + elif child.type == tokens.ATTRIBUTE: + if child.string.startswith("stat."): + file_mode = child.string.split(".")[-1] + value |= getattr(stat, file_mode) + + return value + + +class OsLooseFilePermissions(Rule): + def __init__(self, id: str): + super().__init__( + id=id, + name="incorrect_permission", + description=__doc__, + cwe_id=732, + message="Use of weak hash function '{0}' does not meet security " + "expectations.", + wildcards={ + "os.*": [ + "fchmod", + "chmod", + "lchmod", + "mkdir", + "mkfifo", + "mknod", + "open", + "access", + ] + }, + ) + + def analyze_call(self, context: dict, call: Call) -> Result | None: + if call.name_qualified not in ( + "os.fchmod", + "os.chmod", + "os.lchmod", + "os.mkdir", # default mode=0o777 + "os.mkfifo", # default mode=0o666 + "os.mknod", # default mode=0o600 + "os.open", # default mode=0o777 + "os.access", + ): + return + + argument = call.get_argument( + position=2 if call.name_qualified == "os.open" else 1, + name="mode", + ) + mode = argument.value + + if argument.node.type == "binary_operator": + print(argument.node.children) + mode = resolve_binop(argument.node) + + print(mode) + + if isinstance(mode, int) and _stat_is_dangerous(mode): + return Result( + rule_id=self.id, + location=Location(node=argument.node), + message=self.message.format(mode), + ) + if isinstance(mode, str) and mode.startswith("stat."): + file_mode = mode.split(".")[-1] + mode = getattr(stat, file_mode) + + if _stat_is_dangerous(mode): + return Result( + rule_id=self.id, + location=Location(node=argument.node), + message=self.message.format(mode), + ) diff --git a/setup.cfg b/setup.cfg index 9934d56c..e49af86c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -185,3 +185,6 @@ precli.rules.python = # precli/rules/python/stdlib/hashlib_improper_prng.py PY035 = precli.rules.python.stdlib.hashlib_improper_prng:HashlibImproperPrng + + # precli/rules/python/stdlib/os_loose_file_perm.py + PY036 = precli.rules.python.stdlib.os_loose_file_perm:OsLooseFilePermissions diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_511.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_511.py new file mode 100644 index 00000000..9923ea64 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_511.py @@ -0,0 +1,9 @@ +# level: WARNING +# start_line: 9 +# end_line: 9 +# start_column: 24 +# end_column: 27 +import os + + +os.chmod("/etc/passwd", 511) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o227.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o227.py new file mode 100644 index 00000000..aa7e3cfe --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o227.py @@ -0,0 +1,10 @@ +# level: WARNING +# start_line: 10 +# end_line: 10 +# start_column: 29 +# end_column: 33 +import os + + +mode = 0o227 +os.chmod('/etc/passwd', mode=mode) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o644.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o644.py new file mode 100644 index 00000000..6b992de7 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o644.py @@ -0,0 +1,9 @@ +# level: WARNING +# start_line: 9 +# end_line: 9 +# start_column: 24 +# end_column: 27 +import os + + +os.chmod("/etc/passwd", 0o644) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o7.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o7.py new file mode 100644 index 00000000..f54ea44b --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o7.py @@ -0,0 +1,9 @@ +# level: WARNING +# start_line: 9 +# end_line: 9 +# start_column: 24 +# end_column: 27 +import os + + +os.chmod("/etc/passwd", 0o7) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o755_binop_stat.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o755_binop_stat.py new file mode 100644 index 00000000..5b933a96 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o755_binop_stat.py @@ -0,0 +1,14 @@ +# level: WARNING +# start_line: 16 +# end_line: 16 +# start_column: 24 +# end_column: 27 +import os +import stat + + +# 0o755 for rwxr-xr-x +os.chmod( + "example.txt", + stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR| stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH, +) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o760.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o760.py new file mode 100644 index 00000000..439deb2e --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o760.py @@ -0,0 +1,9 @@ +# level: WARNING +# start_line: 9 +# end_line: 9 +# start_column: 24 +# end_column: 27 +import os + + +os.chmod("/etc/passwd", 0o760) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o770.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o770.py new file mode 100644 index 00000000..c03d57e8 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o770.py @@ -0,0 +1,9 @@ +# level: WARNING +# start_line: 9 +# end_line: 9 +# start_column: 24 +# end_column: 27 +import os + + +os.chmod("/etc/passwd", 0o770) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o776.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o776.py new file mode 100644 index 00000000..9e148101 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o776.py @@ -0,0 +1,9 @@ +# level: WARNING +# start_line: 9 +# end_line: 9 +# start_column: 24 +# end_column: 27 +import os + + +os.chmod("/etc/passwd", 0o776) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_o777.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o777.py new file mode 100644 index 00000000..f002bb6c --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_o777.py @@ -0,0 +1,11 @@ +# level: WARNING +# start_line: 11 +# end_line: 11 +# start_column: 19 +# end_column: 23 +import os + + +filename = '/etc/passwd' +mode = 0o777 +os.chmod(filename, mode) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_stat_S_IXOTH.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_stat_S_IXOTH.py new file mode 100644 index 00000000..60c65658 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_stat_S_IXOTH.py @@ -0,0 +1,10 @@ +# level: WARNING +# start_line: 10 +# end_line: 10 +# start_column: 24 +# end_column: 27 +import os +import stat + + +os.chmod("/etc/passwd", stat.S_IXOTH) diff --git a/tests/unit/rules/python/stdlib/os/examples/os_chmod_x1ff.py b/tests/unit/rules/python/stdlib/os/examples/os_chmod_x1ff.py new file mode 100644 index 00000000..15241e19 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/examples/os_chmod_x1ff.py @@ -0,0 +1,11 @@ +# level: WARNING +# start_line: 11 +# end_line: 11 +# start_column: 19 +# end_column: 23 +import os + + +filename = '/etc/passwd' +mode = 0x1ff +os.chmod(filename, mode) diff --git a/tests/unit/rules/python/stdlib/os/test_os_loose_file_perm.py b/tests/unit/rules/python/stdlib/os/test_os_loose_file_perm.py new file mode 100644 index 00000000..fddceed3 --- /dev/null +++ b/tests/unit/rules/python/stdlib/os/test_os_loose_file_perm.py @@ -0,0 +1,57 @@ +# Copyright 2024 Secure Sauce LLC +import os + +import pytest + +from precli.core.level import Level +from precli.parsers import python +from precli.rules import Rule +from tests.unit.rules import test_case + + +class TestOsLooseFilePermissions(test_case.TestCase): + @classmethod + def setup_class(cls): + cls.rule_id = "PY036" + cls.parser = python.Python() + cls.base_path = os.path.join( + "tests", + "unit", + "rules", + "python", + "stdlib", + "os", + "examples", + ) + + def test_rule_meta(self): + rule = Rule.get_by_id(self.rule_id) + assert rule.id == self.rule_id + assert rule.name == "incorrect_permission" + assert ( + rule.help_url + == f"https://docs.securesauce.dev/rules/{self.rule_id}" + ) + assert rule.default_config.enabled is True + assert rule.default_config.level == Level.WARNING + assert rule.default_config.rank == -1.0 + assert rule.cwe.id == 732 + + @pytest.mark.parametrize( + "filename", + [ + "os_chmod_511.py", + "os_chmod_o227.py", + "os_chmod_o644.py", + "os_chmod_o7.py", + "os_chmod_o755_binop_stat.py", + "os_chmod_o760.py", + "os_chmod_o770.py", + "os_chmod_o776.py", + "os_chmod_o777.py", + "os_chmod_stat_S_IXOTH.py", + "os_chmod_x1ff.py", + ], + ) + def test(self, filename): + self.check(filename)