From 6e21d6949ba3c0b3f1c0b17e2d0de21d9ff41ba2 Mon Sep 17 00:00:00 2001 From: Eric Brown Date: Sat, 28 Sep 2024 18:51:12 -0700 Subject: [PATCH] New rule to check nntplib for use without a timeout PY042 Using nntplib classes NNTP and NNTP_SSL without setting a timeout value will result in using the global default with defaults to None (block forever). Therefore its important to pass a timeout value with initializing the class. Signed-off-by: Eric Brown --- docs/rules.md | 1 + .../rules/python/stdlib/nntplib-no-timeout.md | 10 ++ .../rules/python/stdlib/nntplib_no_timeout.py | 147 ++++++++++++++++++ .../stdlib/nntplib_unverified_context.py | 2 +- setup.cfg | 3 + .../examples/nntplib_nntp_context_mgr.py | 2 +- .../nntplib/examples/nntplib_nntp_login.py | 2 +- .../nntplib/examples/nntplib_nntp_ssl.py | 2 +- .../nntplib_nntp_ssl_context_as_var.py | 6 +- .../examples/nntplib_nntp_ssl_context_none.py | 6 +- .../nntplib_nntp_ssl_context_unset.py | 2 +- .../examples/nntplib_nntp_ssl_no_timeout.py | 14 ++ .../examples/nntplib_nntp_ssl_timeout_5.py | 10 ++ .../nntplib/examples/nntplib_nntp_starttls.py | 2 +- .../nntplib_nntp_starttls_context_as_var.py | 2 +- .../nntplib_nntp_starttls_context_none.py | 2 +- .../nntplib_nntp_starttls_context_unset.py | 2 +- .../examples/nntplib_nntp_timeout_none.py | 15 ++ .../stdlib/nntplib/test_nntplib_no_timeout.py | 49 ++++++ 19 files changed, 264 insertions(+), 15 deletions(-) create mode 100644 docs/rules/python/stdlib/nntplib-no-timeout.md create mode 100644 precli/rules/python/stdlib/nntplib_no_timeout.py create mode 100644 tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_no_timeout.py create mode 100644 tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_timeout_5.py create mode 100644 tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_timeout_none.py create mode 100644 tests/unit/rules/python/stdlib/nntplib/test_nntplib_no_timeout.py diff --git a/docs/rules.md b/docs/rules.md index 7710bbcb..720ff00a 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -65,3 +65,4 @@ | PY039 | [socket — no timeout](rules/python/stdlib/socket-no-timeout.md) | Synchronous Access of `socket` without Timeout | | PY040 | [smtplib — no timeout](rules/python/stdlib/smtplib-no-timeout.md) | Synchronous Access of `SMTP` without Timeout | | PY041 | [imaplib — no timeout](rules/python/stdlib/imaplib-no-timeout.md) | Synchronous Access of `IMAP4` without Timeout | +| PY042 | [nntplib — no timeout](rules/python/stdlib/nntplib-no-timeout.md) | Synchronous Access of `NNTP` without Timeout | diff --git a/docs/rules/python/stdlib/nntplib-no-timeout.md b/docs/rules/python/stdlib/nntplib-no-timeout.md new file mode 100644 index 00000000..14403899 --- /dev/null +++ b/docs/rules/python/stdlib/nntplib-no-timeout.md @@ -0,0 +1,10 @@ +--- +id: PY042 +title: nntplib — no timeout +hide_title: true +pagination_prev: null +pagination_next: null +slug: /rules/PY042 +--- + +::: precli.rules.python.stdlib.nntplib_no_timeout diff --git a/precli/rules/python/stdlib/nntplib_no_timeout.py b/precli/rules/python/stdlib/nntplib_no_timeout.py new file mode 100644 index 00000000..12259168 --- /dev/null +++ b/precli/rules/python/stdlib/nntplib_no_timeout.py @@ -0,0 +1,147 @@ +# Copyright 2024 Secure Sauce LLC +r""" +# Synchronous Access of `NNTP` without Timeout + +The `nntplib.NNTP` and `nntplib.NNTP_SSL` classes are used to connect to +Network News Transfer Protocol (NNTP) servers for accessing Usenet articles. +These classes establish network connections with NNTP servers, and by +default, they do not enforce a timeout on these connections. Without a +timeout, the application may block indefinitely if the NNTP server is slow +or unresponsive, leading to resource exhaustion, Denial of Service (DoS), or +reduced application responsiveness. + +This rule ensures that a timeout parameter is provided when creating +instances of `nntplib.NNTP` or `nntplib.NNTP_SSL` to prevent the risk of +indefinite blocking. + +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="nntplib_nntp_no_timeout.py" +import nntplib +import ssl + + +nntp = nntplib.NNTP("nntp.example.com") +nntp.starttls(ssl.create_default_context()) +``` + +??? example "Example Output" + ``` + > 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 + The class 'nntplib.NNTP' 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 `nntplib.NNTP` or +`nntplib.NNTP_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="nntplib_nntp_no_timeout.py" +import nntplib +import ssl + + +nntp = nntplib.NNTP("nntp.example.com", timeout=5) +nntp.starttls(ssl.create_default_context()) +``` + +# See also + +!!! info + - [nntplib.NNTP — nntplib — IMAP4 protocol client](https://docs.python.org/3/library/nntplib.html#nntplib.NNTP) + - [nntplib.NNTP_SSL — nntplib — IMAP4 protocol client](https://docs.python.org/3/library/nntplib.html#nntplib.NNTP_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 NntplibNoTimeout(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 ( + "nntplib.NNTP", + "nntplib.NNTP_SSL", + ): + return + + if call.name_qualified == "nntplib.NNTP": + # NNTP( + # host, + # port=119, + # user=None, + # password=None, + # readermode=None, + # usenetrc=False, + # timeout=GLOBAL_TIMEOUT + # ) + argument = call.get_argument(position=6, name="timeout") + elif call.name_qualified == "nntplib.NNTP_SSL": + # NNTP_SSL( + # host, + # port=563, + # user=None, + # password=None, + # ssl_context=None, + # readermode=None, + # usenetrc=False, + # timeout=GLOBAL_TIMEOUT + # ) + argument = call.get_argument(position=7, 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, + ) diff --git a/precli/rules/python/stdlib/nntplib_unverified_context.py b/precli/rules/python/stdlib/nntplib_unverified_context.py index c159f71a..73e3da2e 100644 --- a/precli/rules/python/stdlib/nntplib_unverified_context.py +++ b/precli/rules/python/stdlib/nntplib_unverified_context.py @@ -94,7 +94,7 @@ def analyze_call(self, context: dict, call: Call) -> Result | None: return if call.name_qualified == "nntplib.NNTP_SSL": - ssl_context = call.get_argument(name="context") + ssl_context = call.get_argument(name="ssl_context") else: ssl_context = call.get_argument(position=0, name="context") if ssl_context.value is not None: diff --git a/setup.cfg b/setup.cfg index 6e6b9392..69ff7ee1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -206,3 +206,6 @@ precli.rules.python = # precli/rules/python/stdlib/imaplib_no_timeout.py PY041 = precli.rules.python.stdlib.imaplib_no_timeout:ImaplibNoTimeout + + # precli/rules/python/stdlib/nntplib_no_timeout.py + PY042 = precli.rules.python.stdlib.nntplib_no_timeout:NntplibNoTimeout diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_context_mgr.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_context_mgr.py index 2d8e53ba..10f10fcc 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_context_mgr.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_context_mgr.py @@ -6,6 +6,6 @@ import nntplib -with nntplib.NNTP("news.gmane.io") as n: +with nntplib.NNTP("news.gmane.io", timeout=5) as n: n.login("user", "password") n.group("gmane.comp.python.committers") diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_login.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_login.py index c175bfc9..7b72271b 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_login.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_login.py @@ -6,7 +6,7 @@ import nntplib -s = nntplib.NNTP("news.gmane.io") +s = nntplib.NNTP("news.gmane.io", timeout=5) s.login("user", "password") f = open("article.txt", "rb") s.post(f) diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl.py index b17b6c29..a8eac213 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl.py @@ -3,7 +3,7 @@ import ssl -s = nntplib.NNTP_SSL("news.gmane.io", context=ssl.create_default_context()) +s = nntplib.NNTP_SSL("news.gmane.io", ssl_context=ssl.create_default_context(), timeout=5) s.login("user", "password") f = open("article.txt", "rb") s.post(f) diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_as_var.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_as_var.py index 0db2887a..3ffc86bb 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_as_var.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_as_var.py @@ -1,13 +1,13 @@ # level: WARNING # start_line: 10 # end_line: 10 -# start_column: 46 -# end_column: 53 +# start_column: 50 +# end_column: 57 import nntplib context = None -s = nntplib.NNTP_SSL("news.gmane.io", context=context) +s = nntplib.NNTP_SSL("news.gmane.io", ssl_context=context, timeout=5) s.login("user", "password") f = open("article.txt", "rb") s.post(f) diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_none.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_none.py index 4cde4839..59e89534 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_none.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_none.py @@ -1,12 +1,12 @@ # level: WARNING # start_line: 9 # end_line: 9 -# start_column: 46 -# end_column: 50 +# start_column: 50 +# end_column: 54 import nntplib -s = nntplib.NNTP_SSL("news.gmane.io", context=None) +s = nntplib.NNTP_SSL("news.gmane.io", ssl_context=None, timeout=5) s.login("user", "password") f = open("article.txt", "rb") s.post(f) diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_unset.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_unset.py index 58e6fa1a..17645619 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_unset.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_context_unset.py @@ -6,7 +6,7 @@ import nntplib -s = nntplib.NNTP_SSL("news.gmane.io") +s = nntplib.NNTP_SSL("news.gmane.io", timeout=5) s.login("user", "password") f = open("article.txt", "rb") s.post(f) diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_no_timeout.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_no_timeout.py new file mode 100644 index 00000000..04612a1e --- /dev/null +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_no_timeout.py @@ -0,0 +1,14 @@ +# level: WARNING +# start_line: 10 +# end_line: 10 +# start_column: 20 +# end_column: 79 +import nntplib +import ssl + + +s = nntplib.NNTP_SSL("news.gmane.io", ssl_context=ssl.create_default_context()) +s.login("user", "password") +f = open("article.txt", "rb") +s.post(f) +s.quit() diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_timeout_5.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_timeout_5.py new file mode 100644 index 00000000..a8eac213 --- /dev/null +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_ssl_timeout_5.py @@ -0,0 +1,10 @@ +# level: NONE +import nntplib +import ssl + + +s = nntplib.NNTP_SSL("news.gmane.io", ssl_context=ssl.create_default_context(), timeout=5) +s.login("user", "password") +f = open("article.txt", "rb") +s.post(f) +s.quit() diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls.py index fc53537d..d76932ec 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls.py @@ -3,7 +3,7 @@ import ssl -s = nntplib.NNTP("news.gmane.io") +s = nntplib.NNTP("news.gmane.io", timeout=5) s.starttls(context=ssl.create_default_context()) s.login("user", "password") f = open("article.txt", "rb") diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_as_var.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_as_var.py index dc9885dd..e920a627 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_as_var.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_as_var.py @@ -7,7 +7,7 @@ context = None -s = nntplib.NNTP("news.gmane.io") +s = nntplib.NNTP("news.gmane.io", timeout=5) s.starttls(context=context) s.login("user", "password") f = open("article.txt", "rb") diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_none.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_none.py index aa6258b0..36f8f200 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_none.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_none.py @@ -6,7 +6,7 @@ import nntplib -s = nntplib.NNTP("news.gmane.io") +s = nntplib.NNTP("news.gmane.io", timeout=5) s.starttls(context=None) s.login("user", "password") f = open("article.txt", "rb") diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_unset.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_unset.py index 035308ca..765c92eb 100644 --- a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_unset.py +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_starttls_context_unset.py @@ -6,7 +6,7 @@ import nntplib -s = nntplib.NNTP("news.gmane.io") +s = nntplib.NNTP("news.gmane.io", timeout=5) s.starttls() s.login("user", "password") f = open("article.txt", "rb") diff --git a/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_timeout_none.py b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_timeout_none.py new file mode 100644 index 00000000..f0e0ba01 --- /dev/null +++ b/tests/unit/rules/python/stdlib/nntplib/examples/nntplib_nntp_timeout_none.py @@ -0,0 +1,15 @@ +# level: WARNING +# start_line: 10 +# end_line: 10 +# start_column: 42 +# end_column: 46 +import nntplib +import ssl + + +s = nntplib.NNTP("news.gmane.io", timeout=None) +s.starttls(context=ssl.create_default_context()) +s.login("user", "password") +f = open("article.txt", "rb") +s.post(f) +s.quit() diff --git a/tests/unit/rules/python/stdlib/nntplib/test_nntplib_no_timeout.py b/tests/unit/rules/python/stdlib/nntplib/test_nntplib_no_timeout.py new file mode 100644 index 00000000..6709248c --- /dev/null +++ b/tests/unit/rules/python/stdlib/nntplib/test_nntplib_no_timeout.py @@ -0,0 +1,49 @@ +# 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 TestNntplibNoTimeout(test_case.TestCase): + @classmethod + def setup_class(cls): + cls.rule_id = "PY042" + cls.parser = python.Python() + cls.base_path = os.path.join( + "tests", + "unit", + "rules", + "python", + "stdlib", + "nntplib", + "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", + [ + "nntplib_nntp_ssl_no_timeout.py", + "nntplib_nntp_ssl_timeout_5.py", + "nntplib_nntp_timeout_none.py", + ], + ) + def test(self, filename): + self.check(filename)