Skip to content

Commit

Permalink
New rule to check nntplib for use without a timeout
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
ericwb committed Sep 29, 2024
1 parent af288b6 commit 6e21d69
Show file tree
Hide file tree
Showing 19 changed files with 264 additions and 15 deletions.
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
10 changes: 10 additions & 0 deletions docs/rules/python/stdlib/nntplib-no-timeout.md
Original file line number Diff line number Diff line change
@@ -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
147 changes: 147 additions & 0 deletions precli/rules/python/stdlib/nntplib_no_timeout.py
Original file line number Diff line number Diff line change
@@ -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,
)
2 changes: 1 addition & 1 deletion precli/rules/python/stdlib/nntplib_unverified_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
Original file line number Diff line number Diff line change
@@ -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()
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
49 changes: 49 additions & 0 deletions tests/unit/rules/python/stdlib/nntplib/test_nntplib_no_timeout.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 6e21d69

Please sign in to comment.