Skip to content

Commit

Permalink
Add new rule around loose permission in os module
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
ericwb committed Aug 23, 2024
1 parent cf6952c commit f2db582
Show file tree
Hide file tree
Showing 14 changed files with 349 additions and 0 deletions.
179 changes: 179 additions & 0 deletions precli/rules/python/stdlib/os_loose_file_perm.py
Original file line number Diff line number Diff line change
@@ -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),
)
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_511.py
Original file line number Diff line number Diff line change
@@ -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)
10 changes: 10 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_o227.py
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 9 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_o644.py
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 9 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_o7.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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,
)
9 changes: 9 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_o760.py
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 9 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_o770.py
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 9 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_o776.py
Original file line number Diff line number Diff line change
@@ -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)
11 changes: 11 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_o777.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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)
11 changes: 11 additions & 0 deletions tests/unit/rules/python/stdlib/os/examples/os_chmod_x1ff.py
Original file line number Diff line number Diff line change
@@ -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)
57 changes: 57 additions & 0 deletions tests/unit/rules/python/stdlib/os/test_os_loose_file_perm.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit f2db582

Please sign in to comment.