Skip to content

Commit

Permalink
New Python rule to check poplib use without timeout (#606)
Browse files Browse the repository at this point in the history
PY043

The poplib.POP3 and POP3_SSL classes have a timeout argument that
defaults to the socket global timeout which defaults to None. So a
timeout should be set to avoid the POP3 connection from blocking
forever.

Signed-off-by: Eric Brown <[email protected]>
  • Loading branch information
ericwb authored Sep 29, 2024
1 parent f86ea40 commit 44d170f
Show file tree
Hide file tree
Showing 20 changed files with 228 additions and 24 deletions.
2 changes: 1 addition & 1 deletion precli/rules/python/stdlib/nntplib_no_timeout.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
```
> precli tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_no_timeout.py
⚠️ Warning on line 10 in tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_no_timeout.py
PY041: Synchronous Access of Remote Resource without Timeout
PY042: Synchronous Access of Remote Resource without Timeout
The class 'nntplib.NNTP' is used without a timeout, which may cause the application to block indefinitely if the remote server does not respond.
```
Expand Down
136 changes: 136 additions & 0 deletions precli/rules/python/stdlib/poplib_no_timeout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Copyright 2024 Secure Sauce LLC
r"""
# Synchronous Access of `POP3` without Timeout
The `poplib.POP3` and `poplib.POP3_SSL` classes are used to connect to mail
servers using the Post Office Protocol version 3 (POP3) for retrieving emails.
By default, these classes do not enforce a timeout on the network connection,
which means that an application could block indefinitely if the mail server
becomes unresponsive or there is a network failure. This can result in resource
exhaustion, Denial of Service (DoS), or unresponsive behavior in the
application.
This rule ensures that a timeout parameter is provided when creating instances
of `poplib.POP3` or `poplib.POP3_SSL` to prevent the risk of indefinite
blocking during network communication.
Failing to specify a timeout in these classes may cause the application to
block indefinitely while waiting for a response from the mail server. This can
lead to Denial of Service (DoS) vulnerabilities or cause the application to
become unresponsive.
# Example
```python linenums="1" hl_lines="5" title="poplib_pop3_no_timeout.py"
import poplib
import ssl
pop = poplib.POP3("mail.my-mail-server.com")
pop.stls(ssl.create_default_context())
```
??? example "Example Output"
```
> precli tests/unit/rules/python/stdlib/poplib/examples/poplib_pop3_no_timeout.py
⚠️ Warning on line 10 in tests/unit/rules/python/stdlib/poplib/examples/poplib_pop3_no_timeout.py
PY043: Synchronous Access of Remote Resource without Timeout
The class 'poplib.POP3' is used without a timeout, which may cause the application to block indefinitely if the remote server does not respond.
```
# Remediation
Always provide a timeout parameter when using `poplib.POP3` or
`poplib.POP3_SSL`. This ensures that if the mail server is unreachable or
unresponsive, the connection attempt will fail after a set period, preventing
indefinite blocking and resource exhaustion.
```python linenums="1" hl_lines="5" title="poplib_pop3_no_timeout.py"
import poplib
import ssl
pop = poplib.POP3("mail.my-mail-server.com", timeout=5)
pop.stls(ssl.create_default_context())
```
# See also
!!! info
- [poplib.POP3 — poplib — POP3 protocol client](https://docs.python.org/3/library/poplib.html#poplib.POP3)
- [poplib.POP3_SSL — poplib — POP3 protocol client](https://docs.python.org/3/library/poplib.html#poplib.POP3_SSL)
- [CWE-1088: Synchronous Access of Remote Resource without Timeout](https://cwe.mitre.org/data/definitions/1088.html)
_New in version 0.6.7_
""" # noqa: E501
from precli.core.call import Call
from precli.core.location import Location
from precli.core.result import Result
from precli.rules import Rule


class PoplibNoTimeout(Rule):
def __init__(self, id: str):
super().__init__(
id=id,
name="no_timeout",
description=__doc__,
cwe_id=1088,
message="The class '{0}' is used without a timeout, which may "
"cause the application to block indefinitely if the remote server "
"does not respond.",
)

def analyze_call(self, context: dict, call: Call) -> Result | None:
if call.name_qualified not in (
"poplib.POP3",
"poplib.POP3_SSL",
):
return

if call.name_qualified == "poplib.POP3":
# POP3(host, port=110, timeout=GLOBAL_TIMEOUT)
argument = call.get_argument(position=2, name="timeout")
elif call.name_qualified == "poplib.POP3_SSL":
# POP3_SSL(
# host,
# port=995,
# *,
# timeout=GLOBAL_TIMEOUT,
# context=None,
# )
argument = call.get_argument(name="timeout")

timeout = argument.value

if argument.node is None:
arg_list_node = call.arg_list_node
fix_node = arg_list_node
args = [child.string for child in arg_list_node.named_children]
args.append("timeout=5")
content = f"({', '.join(args)})"
result_node = call.arg_list_node
elif timeout is None:
fix_node = argument.node
result_node = argument.node
content = "5"
else:
# If the timeout parameter is set to be zero, the class will raise
# a ValueError to prevent the creation of a non-blocking socket. A
# negative value also raises ValueError. So there is no need to
# check for these values.
return

fixes = Rule.get_fixes(
context=context,
deleted_location=Location(fix_node),
description="Set timeout parameter to a small number of seconds.",
inserted_content=content,
)
return Result(
rule_id=self.id,
location=Location(node=result_node),
message=self.message.format(call.name_qualified),
fixes=fixes,
)
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,6 @@ precli.rules.python =

# precli/rules/python/stdlib/nntplib_no_timeout.py
PY042 = precli.rules.python.stdlib.nntplib_no_timeout:NntplibNoTimeout

# precli/rules/python/stdlib/poplib_no_timeout.py
PY043 = precli.rules.python.stdlib.poplib_no_timeout:PoplibNoTimeout
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3("localhost")
M = poplib.POP3("localhost", 110, 5)
secret = getpass.getpass()
M.apop(getpass.getuser(), secret)
numMessages = len(M.list()[1])
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# level: WARNING
# start_line: 10
# end_line: 10
# start_column: 17
# end_column: 44
import poplib
import ssl


pop = poplib.POP3("mail.my-mail-server.com")
pop.stls(ssl.create_default_context())
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3("localhost")
M = poplib.POP3("localhost", timeout=5)
M.pass_(getpass.getpass())
numMessages = len(M.list()[1])
for i in range(numMessages):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3("localhost")
M = poplib.POP3("localhost", timeout=5)
M.rpop(getpass.getuser())
numMessages = len(M.list()[1])
for i in range(numMessages):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import ssl


M = poplib.POP3_SSL("localhost", context=ssl.create_default_context())
M = poplib.POP3_SSL("localhost", context=ssl.create_default_context(), timeout=5)
M.user(getpass.getuser())
M.pass_(getpass.getpass())
numMessages = len(M.list()[1])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@


context = None
M = poplib.POP3_SSL("localhost", context=context)
M = poplib.POP3_SSL("localhost", context=context, timeout=5)
M.user(getpass.getuser())
M.pass_(getpass.getpass())
numMessages = len(M.list()[1])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3_SSL("localhost", context=None)
M = poplib.POP3_SSL("localhost", context=None, timeout=5)
M.user(getpass.getuser())
M.pass_(getpass.getpass())
numMessages = len(M.list()[1])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3_SSL("localhost")
M = poplib.POP3_SSL("localhost", timeout=5)
M.user(getpass.getuser())
M.pass_(getpass.getpass())
numMessages = len(M.list()[1])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import ssl


M = poplib.POP3("localhost")
M = poplib.POP3("localhost", timeout=5)
M.stls(context=ssl.create_default_context())
M.user(getpass.getuser())
M.pass_(getpass.getpass())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@


context = None
M = poplib.POP3("localhost")
M = poplib.POP3("localhost", timeout=5)
M.stls(context=context)
M.user(getpass.getuser())
M.pass_(getpass.getpass())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3("localhost")
M = poplib.POP3("localhost", timeout=5)
M.stls(context=None)
M.user(getpass.getuser())
M.pass_(getpass.getpass())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3("localhost")
M = poplib.POP3("localhost", timeout=5)
M.stls()
M.user(getpass.getuser())
M.pass_(getpass.getpass())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# level: WARNING
# start_line: 11
# end_line: 11
# start_column: 79
# end_column: 83
import getpass
import poplib
import ssl


M = poplib.POP3_SSL("localhost", context=ssl.create_default_context(), timeout=None)
M.user(getpass.getuser())
M.pass_(getpass.getpass())
numMessages = len(M.list()[1])
for i in range(numMessages):
for j in M.retr(i + 1)[1]:
print(j)
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import poplib


M = poplib.POP3("localhost")
M = poplib.POP3("localhost", timeout=5)
M.user(getpass.getuser())
numMessages = len(M.list()[1])
for i in range(numMessages):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ def test_rule_meta(self):
"filename",
[
"poplib_pop3_apop.py",
"poplib_pop3_context_mgr.py",
"poplib_pop3_pass_.py",
"poplib_pop3_rpop.py",
"poplib_pop3_ssl.py",
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/rules/python/stdlib/poplib/test_poplib_no_timeout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# 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 TestPoplibNoTimeout(test_case.TestCase):
@classmethod
def setup_class(cls):
cls.rule_id = "PY043"
cls.parser = python.Python()
cls.base_path = os.path.join(
"tests",
"unit",
"rules",
"python",
"stdlib",
"poplib",
"examples",
)

def test_rule_meta(self):
rule = Rule.get_by_id(self.rule_id)
assert rule.id == self.rule_id
assert rule.name == "no_timeout"
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 == 1088

@pytest.mark.parametrize(
"filename",
[
"poplib_pop3_no_timeout.py",
"poplib_pop3_timeout_none.py",
],
)
def test(self, filename):
self.check(filename)

0 comments on commit 44d170f

Please sign in to comment.