Skip to content

Commit

Permalink
New rule for pathlib.Path usage with loose permissions (#569)
Browse files Browse the repository at this point in the history
This rule adds checks for usage of pathlib.Path functions where a file
mode can be passed and the file mode is considered loose permissions. It
checks functions chmod, lchmod, mkdir, and touch.

Unit tests and documentation also added.

Closes: #217

Signed-off-by: Eric Brown <[email protected]>
  • Loading branch information
ericwb authored Aug 26, 2024
1 parent e8fbe74 commit 74ff6f3
Show file tree
Hide file tree
Showing 24 changed files with 453 additions and 5 deletions.
3 changes: 2 additions & 1 deletion docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,5 @@
| PY033 | [re — denial of service](rules/python/stdlib/re-denial-of-service.md) | Inefficient Regular Expression Complexity in `re` Module |
| PY034 | [hmac — weak key](rules/python/stdlib/hmac-weak-key.md) | Insufficient `hmac` Key Size |
| PY035 | [hashlib — improper prng](rules/python/stdlib/hashlib-improper-prng.md) | Improper Randomness for Cryptographic `hashlib` Functions |
| PY036 | [hashlib — improper prng](rules/python/stdlib/os-loose-file-perm.md) | Incorrect Permission Assignment for Critical Resource using `os` Module |
| PY036 | [os — incorrect permission](rules/python/stdlib/os-loose-file-perm.md) | Incorrect Permission Assignment for Critical Resource using `os` Module |
| PY037 | [pathlib — incorrect permission](rules/python/stdlib/pathlib-loose-file-perm.md) | Incorrect Permission Assignment for Critical Resource using `pathlib` Module |
10 changes: 10 additions & 0 deletions docs/rules/python/stdlib/pathlib-loose-file-perm.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
id: PY037
title: pathlib — incorrect permission
hide_title: true
pagination_prev: null
pagination_next: null
slug: /rules/PY037
---

::: precli.rules.python.stdlib.pathlib_loose_file_perm
9 changes: 5 additions & 4 deletions precli/rules/python/stdlib/os_loose_file_perm.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
??? example "Example Output"
```
> precli tests/unit/rules/python/stdlib/argparse/examples/os_chmod_o755_binop_stat.py
⚠️ Warning on line 13 in tests/unit/rules/python/stdlib/os/examples/os_chmod_o755_binop_stat.py
> precli tests/unit/rules/python/stdlib/os/examples/os_chmod_o755_binop_stat.py
⚠️ Warning on line 8 in tests/unit/rules/python/stdlib/os/examples/os_chmod_o755_binop_stat.py
PY036: Incorrect Permission Assignment for Critical Resource
Mode '0o755' grants excessive permissions, potentially allowing unauthorized access or modification.
```
Expand Down Expand Up @@ -161,8 +161,9 @@ def analyze_call(self, context: dict, call: Call) -> Result | None:
mode = 0o666
location = Location(node=call.arg_list_node)
message = (
f"The default mode value of {oct(mode)} is overly permissive,"
" potentially allowing unauthorized access or modification."
f"The default mode value of '{oct(mode)}' is overly "
"permissive, potentially allowing unauthorized access or "
"modification."
)

if isinstance(mode, int) and self._risky_mode(mode):
Expand Down
172 changes: 172 additions & 0 deletions precli/rules/python/stdlib/pathlib_loose_file_perm.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
# Copyright 2024 Secure Sauce LLC
r"""
# Incorrect Permission Assignment for Critical Resource using `pathlib` Module
This rule identifies instances in code where potentially risky file or
directory permission modes are being set using functions like chmod, lchmod,
mkdir, touch, and similar system calls. Setting inappropriate permission modes
can lead to security vulnerabilities, including unauthorized access, data
leakage, or privilege escalation.
Setting overly permissive modes (e.g., 0777, 0666) can expose files or
directories to unauthorized access or modification. The rule flags instances
where the mode may pose a security risk, particularly when:
- Write permissions are granted to others (group or world): Modes like 0666
(read/write for everyone) or 0777 (read/write/execute for everyone) are
inherently dangerous.
- Inappropriate permissions for sensitive files: Configuration files,
credential files, and other sensitive files should not be globally readable
or writable.
## Examples
```python linenums="1" hl_lines="8-9" title="pathlib_chmod_o755_binop_stat.py"
import pathlib
import stat
# 0o755 for rwxr-xr-x
file_path = pathlib.Path("example.txt")
file_path.chmod(
stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR | stat.S_IRGRP | stat.S_IXGRP |
stat.S_IROTH | stat.S_IXOTH
)
```
??? example "Example Output"
```
> precli tests/unit/rules/python/stdlib/pathlib/examples/pathlib_chmod_o755_binop_stat.py
⚠️ Warning on line 8 in tests/unit/rules/python/stdlib/pathlib/examples/pathlib_chmod_o755_binop_stat.py
PY036: Incorrect Permission Assignment for Critical Resource
Mode '0o755' grants excessive permissions, potentially allowing unauthorized access or modification.
```
## Remediation
- Restrict file permissions: Use more restrictive permission modes that limit
access to only the necessary users.
- Review file sensitivity: Ensure that sensitive files are protected with
the appropriate permissions.
- Apply the principle of least privilege: Only grant the minimum required
permissions for the intended functionality.
Safer Permissions Examples:
- For general files: 0644 (read/write for owner, read-only for group and
others)
- For sensitive files: 0600 (read/write for owner only)
- For executable scripts: 0755 (read/write/execute for owner, read/execute
for group and others)
```python linenums="1" hl_lines="8" title="os_chmod_o755_binop_stat.py"
import pathlib
import stat
# 0o644 for rw-r--r--
file_path = pathlib.Path("example.txt")
file_path.chmod(
stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
)
```
## See also
!!! info
- [pathlib — Object-oriented filesystem paths — Python documentation](https://docs.python.org/3/library/pathlib.html#pathlib.Path.chmod)
- [stat — Interpreting stat() results — Python documentation](https://docs.python.org/3/library/stat.html#stat.S_ISUID)
- [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.level import Level
from precli.core.location import Location
from precli.core.result import Result
from precli.parsers.node_types import NodeTypes
from precli.rules import Rule


class PathlibLooseFilePermissions(Rule):
def __init__(self, id: str):
super().__init__(
id=id,
name="incorrect_permission",
description=__doc__,
cwe_id=732,
message="Mode '{0}' grants excessive permissions, potentially "
"allowing unauthorized access or modification.",
)

def _risky_mode(self, mode):
return (
mode & stat.S_IWOTH
or mode & stat.S_IWGRP
or mode & stat.S_IXGRP
or mode & stat.S_IXOTH
)

def analyze_import_statement(
self, context: dict, package: str, alias: str
) -> None:
if package == "stat":
for constant in dir(stat):
if constant.startswith("S_"):
context["symtab"].put(
f"{alias}.{constant}",
NodeTypes.IDENTIFIER,
getattr(stat, constant),
)

def analyze_import_from_statement(
self, context: dict, package: str, alias: str
) -> None:
if package.startswith("stat."):
constant = package.split(".")[-1]
context["symtab"].put(
alias,
NodeTypes.IDENTIFIER,
getattr(stat, constant),
)

def analyze_call(self, context: dict, call: Call) -> Result | None:
if call.name_qualified not in (
"pathlib.Path.chmod",
"pathlib.Path.lchmod",
"pathlib.Path.mkdir", # default mode=0o777
"pathlib.Path.touch", # default mode=0o666
):
return

argument = call.get_argument(position=0, name="mode")
mode = argument.value

if argument.node is not None:
location = Location(node=argument.node)
message = self.message
elif call.name_qualified in (
"pathlib.Path.mkdir",
"pathlib.Path.touch",
):
if call.name_qualified == "pathlib.Path.mkdir":
mode = 0o777
elif call.name_qualified == "pathlib.Path.touch":
mode = 0o666
location = Location(node=call.arg_list_node)
message = (
f"The default mode value of '{oct(mode)}' is overly "
"permissive, potentially allowing unauthorized access or "
"modification."
)

if isinstance(mode, int) and self._risky_mode(mode):
return Result(
rule_id=self.id,
location=location,
level=Level.ERROR if mode & stat.S_IWOTH else Level.WARNING,
message=message.format(oct(mode)),
)
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,6 @@ precli.rules.python =

# precli/rules/python/stdlib/os_loose_file_perm.py
PY036 = precli.rules.python.stdlib.os_loose_file_perm:OsLooseFilePermissions

# precli/rules/python/stdlib/pathlib_loose_file_perm.py
PY037 = precli.rules.python.stdlib.pathlib_loose_file_perm:PathlibLooseFilePermissions
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# level: WARNING
# start_line: 11
# end_line: 11
# start_column: 16
# end_column: 21
from pathlib import Path
from stat import S_IXOTH as IXOTH


file_path = Path("example.sh")
file_path.chmod(IXOTH)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# level: WARNING
# start_line: 11
# end_line: 11
# start_column: 16
# end_column: 23
import pathlib
from stat import S_IXOTH


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(S_IXOTH)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# level: WARNING
# start_line: 11
# end_line: 11
# start_column: 16
# end_column: 25
import pathlib
import stat as S


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(S.S_IXOTH)
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# level: WARNING
# start_line: 12
# end_line: 12
# start_column: 16
# end_column: 20
from pathlib import Path
from stat import *


file_path = Path("/etc/passwd")
mode = S_IXUSR | S_IXGRP | S_IXOTH
file_path.chmod(mode)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# level: NONE
import pathlib


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(0o644)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# level: ERROR
# start_line: 10
# end_line: 10
# start_column: 16
# end_column: 19
import pathlib


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(0o7)
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# level: WARNING
# start_line: 13
# end_line: 14
# start_column: 4
# end_column: 31
import pathlib
import stat


# 0o755 for rwxr-xr-x
file_path = pathlib.Path("example.txt")
file_path.chmod(
stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR | stat.S_IRGRP | stat.S_IXGRP |
stat.S_IROTH | stat.S_IXOTH
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# level: WARNING
# start_line: 10
# end_line: 10
# start_column: 16
# end_column: 21
import pathlib


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(0o760)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# level: WARNING
# start_line: 10
# end_line: 10
# start_column: 16
# end_column: 21
import pathlib


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(0o770)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# level: ERROR
# start_line: 10
# end_line: 10
# start_column: 16
# end_column: 21
import pathlib


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(0o776)
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# level: ERROR
# start_line: 12
# end_line: 12
# start_column: 21
# end_column: 25
import pathlib


filename = "/etc/passwd"
mode = 0o777
file_path = pathlib.Path(filename)
file_path.chmod(mode=mode)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# level: WARNING
# start_line: 11
# end_line: 11
# start_column: 16
# end_column: 28
import pathlib
import stat


file_path = pathlib.Path("/etc/passwd")
file_path.chmod(stat.S_IXOTH)
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# level: ERROR
# start_line: 12
# end_line: 12
# start_column: 16
# end_column: 20
from pathlib import Path


filename = '/etc/passwd'
mode = 0x1ff
file_path = Path(filename)
file_path.chmod(mode)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# level: ERROR
# start_line: 11
# end_line: 11
# start_column: 22
# end_column: 26
from pathlib import Path


mode = 0o227
file_path = Path('/etc/passwd')
file_path.lchmod(mode=mode)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# level: ERROR
# start_line: 10
# end_line: 10
# start_column: 15
# end_column: 17
from pathlib import Path


file_path = Path("examples")
file_path.mkdir()
Loading

0 comments on commit 74ff6f3

Please sign in to comment.