Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(plugins): add support for httpx in B113 #1060

Merged
merged 9 commits into from
Jun 25, 2024
Merged

feat(plugins): add support for httpx in B113 #1060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5623f06
refactor: move `HTTP_VERBS`/`HTTPX_ATTRS` to `core.utils`
mkniewallner Sep 19, 2023
277b04c
perf(core): use sets for `HTTP_REQUEST_VERBS`/`HTTPX_ATTRS`
mkniewallner Sep 19, 2023
77d2901
feat(plugins): add support for `httpx` in `B113`
mkniewallner Sep 19, 2023
4fc4f19
refactor: put back `HTTP_VERBS`/`HTTPX_ATTRS` into plugins
mkniewallner Sep 22, 2023
8e26f94
Merge branch 'main' into feat/add-httpx-to-B113
ericwb Sep 24, 2023
2e2e442
Merge branch 'main' into feat/add-httpx-to-B113
ericwb Jun 24, 2024
0ee786c
Update bandit/plugins/request_without_timeout.py
ericwb Jun 25, 2024
9920484
Update bandit/plugins/request_without_timeout.py
ericwb Jun 25, 2024
a1e7d7f
Update bandit/plugins/request_without_timeout.py
ericwb Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bandit/plugins/crypto_request_no_cert_validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
@test.checks("Call")
@test.test_id("B501")
def request_with_no_cert_validation(context):
HTTP_VERBS = ("get", "options", "head", "post", "put", "patch", "delete")
HTTPX_ATTRS = ("request", "stream", "Client", "AsyncClient") + HTTP_VERBS
HTTP_VERBS = {"get", "options", "head", "post", "put", "patch", "delete"}
HTTPX_ATTRS = {"request", "stream", "Client", "AsyncClient"} | HTTP_VERBS
qualname = context.call_function_name_qual.split(".")[0]

if (
Expand Down
24 changes: 17 additions & 7 deletions bandit/plugins/request_without_timeout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
B113: Test for missing requests timeout
=======================================

This plugin test checks for ``requests`` calls without a timeout specified.
This plugin test checks for ``requests`` or ``httpx`` calls without a timeout
specified.

Nearly all production code should use this parameter in nearly all requests,
Failure to do so can cause your program to hang indefinitely.
Expand All @@ -17,7 +18,7 @@

.. code-block:: none

>> Issue: [B113:request_without_timeout] Requests call without timeout
>> Issue: [B113:request_without_timeout] Call to requests without timeout
Severity: Medium Confidence: Low
CWE: CWE-400 (https://cwe.mitre.org/data/definitions/400.html)
More Info: https://bandit.readthedocs.io/en/latest/plugins/b113_request_without_timeout.html
Expand All @@ -27,7 +28,7 @@
4 requests.get('https://gmail.com', timeout=None)

--------------------------------------------------
>> Issue: [B113:request_without_timeout] Requests call with timeout set to None
>> Issue: [B113:request_without_timeout] Call to requests with timeout set to None
Severity: Medium Confidence: Low
CWE: CWE-400 (https://cwe.mitre.org/data/definitions/400.html)
More Info: https://bandit.readthedocs.io/en/latest/plugins/b113_request_without_timeout.html
Expand All @@ -42,6 +43,9 @@

.. versionadded:: 1.7.5

.. versionchanged:: 1.7.10
Added check for httpx module

""" # noqa: E501
import bandit
from bandit.core import issue
Expand All @@ -51,23 +55,29 @@
@test.checks("Call")
@test.test_id("B113")
def request_without_timeout(context):
http_verbs = ("get", "options", "head", "post", "put", "patch", "delete")
HTTP_VERBS = {"get", "options", "head", "post", "put", "patch", "delete"}
HTTPX_ATTRS = {"request", "stream", "Client", "AsyncClient"} | HTTP_VERBS
qualname = context.call_function_name_qual.split(".")[0]

if qualname == "requests" and context.call_function_name in http_verbs:
if (
qualname == "requests"
and context.call_function_name in HTTP_VERBS
or qualname == "httpx"
and context.call_function_name in HTTPX_ATTRS
):
# check for missing timeout
if context.check_call_arg_value("timeout") is None:
return bandit.Issue(
severity=bandit.MEDIUM,
confidence=bandit.LOW,
cwe=issue.Cwe.UNCONTROLLED_RESOURCE_CONSUMPTION,
text="Requests call without timeout",
text=f"Call to {qualname} without timeout",
)
# check for timeout=None
if context.check_call_arg_value("timeout", "None"):
return bandit.Issue(
severity=bandit.MEDIUM,
confidence=bandit.LOW,
cwe=issue.Cwe.UNCONTROLLED_RESOURCE_CONSUMPTION,
text="Requests call with timeout set to None",
text=f"Call to {qualname} with timeout set to None",
)
55 changes: 48 additions & 7 deletions examples/requests-missing-timeout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,68 @@
import httpx
import requests
import not_requests

# Errors
requests.get('https://gmail.com')
requests.get('https://gmail.com', timeout=None)
requests.get('https://gmail.com', timeout=5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't remove these. They verify there's no regression when someone passes a timeout value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They have not been removed but moved under "Okay" section below, since those cases did not trigger violations.

requests.post('https://gmail.com')
requests.post('https://gmail.com', timeout=None)
requests.post('https://gmail.com', timeout=5)
requests.put('https://gmail.com')
requests.put('https://gmail.com', timeout=None)
requests.put('https://gmail.com', timeout=5)
requests.delete('https://gmail.com')
requests.delete('https://gmail.com', timeout=None)
requests.delete('https://gmail.com', timeout=5)
requests.patch('https://gmail.com')
requests.patch('https://gmail.com', timeout=None)
requests.patch('https://gmail.com', timeout=5)
requests.options('https://gmail.com')
requests.options('https://gmail.com', timeout=None)
requests.options('https://gmail.com', timeout=5)
requests.head('https://gmail.com')
requests.head('https://gmail.com', timeout=None)
requests.head('https://gmail.com', timeout=5)
httpx.get('https://gmail.com')
httpx.get('https://gmail.com', timeout=None)
httpx.post('https://gmail.com')
httpx.post('https://gmail.com', timeout=None)
httpx.put('https://gmail.com')
httpx.put('https://gmail.com', timeout=None)
httpx.delete('https://gmail.com')
httpx.delete('https://gmail.com', timeout=None)
httpx.patch('https://gmail.com')
httpx.patch('https://gmail.com', timeout=None)
httpx.options('https://gmail.com')
httpx.options('https://gmail.com', timeout=None)
httpx.head('https://gmail.com')
httpx.head('https://gmail.com', timeout=None)
httpx.Client()
httpx.Client(timeout=None)
httpx.AsyncClient()
httpx.AsyncClient(timeout=None)
with httpx.Client() as client:
client.get('https://gmail.com')
with httpx.Client(timeout=None) as client:
client.get('https://gmail.com')
async with httpx.AsyncClient() as client:
await client.get('https://gmail.com')
async with httpx.AsyncClient(timeout=None) as client:
await client.get('https://gmail.com')

# Okay
not_requests.get('https://gmail.com')
requests.get('https://gmail.com', timeout=5)
requests.post('https://gmail.com', timeout=5)
requests.put('https://gmail.com', timeout=5)
requests.delete('https://gmail.com', timeout=5)
requests.patch('https://gmail.com', timeout=5)
requests.options('https://gmail.com', timeout=5)
requests.head('https://gmail.com', timeout=5)
httpx.get('https://gmail.com', timeout=5)
httpx.post('https://gmail.com', timeout=5)
httpx.put('https://gmail.com', timeout=5)
httpx.delete('https://gmail.com', timeout=5)
httpx.patch('https://gmail.com', timeout=5)
httpx.options('https://gmail.com', timeout=5)
httpx.head('https://gmail.com', timeout=5)
httpx.Client(timeout=5)
httpx.AsyncClient(timeout=5)
with httpx.Client(timeout=5) as client:
client.get('https://gmail.com')
async with httpx.AsyncClient(timeout=5) as client:
await client.get('https://gmail.com')
46 changes: 24 additions & 22 deletions examples/requests-ssl-verify-disabled.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import httpx
import requests

# Errors
requests.get('https://gmail.com', timeout=30, verify=True)
requests.get('https://gmail.com', timeout=30, verify=False)
requests.post('https://gmail.com', timeout=30, verify=True)
Expand All @@ -16,25 +17,26 @@
requests.head('https://gmail.com', timeout=30, verify=True)
requests.head('https://gmail.com', timeout=30, verify=False)

httpx.request('GET', 'https://gmail.com', verify=True)
httpx.request('GET', 'https://gmail.com', verify=False)
httpx.get('https://gmail.com', verify=True)
httpx.get('https://gmail.com', verify=False)
httpx.options('https://gmail.com', verify=True)
httpx.options('https://gmail.com', verify=False)
httpx.head('https://gmail.com', verify=True)
httpx.head('https://gmail.com', verify=False)
httpx.post('https://gmail.com', verify=True)
httpx.post('https://gmail.com', verify=False)
httpx.put('https://gmail.com', verify=True)
httpx.put('https://gmail.com', verify=False)
httpx.patch('https://gmail.com', verify=True)
httpx.patch('https://gmail.com', verify=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please just add a separate file rather than abuse this one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the check for missing timeouts on httpx made this test file report medium violations in the test due to missing timeout values. I did that with the intention of keeping only the violations related to SSL verification disabled. Would you prefer to keep the file as is and bump the number of reported violations here instead?

For the missing timeouts, I can still create a separate test case and example file if you prefer though. I wanted to stay consistent with what has been done in #861, but having a separate test case would probably be preferrable.

httpx.delete('https://gmail.com', verify=True)
httpx.delete('https://gmail.com', verify=False)
httpx.stream('https://gmail.com', verify=True)
httpx.stream('https://gmail.com', verify=False)
httpx.Client()
httpx.Client(verify=False)
httpx.AsyncClient()
httpx.AsyncClient(verify=False)
# Okay
httpx.request('GET', 'https://gmail.com', timeout=30, verify=True)
httpx.request('GET', 'https://gmail.com', timeout=30, verify=False)
httpx.get('https://gmail.com', timeout=30, verify=True)
httpx.get('https://gmail.com', timeout=30, verify=False)
httpx.options('https://gmail.com', timeout=30, verify=True)
httpx.options('https://gmail.com', timeout=30, verify=False)
httpx.head('https://gmail.com', timeout=30, verify=True)
httpx.head('https://gmail.com', timeout=30, verify=False)
httpx.post('https://gmail.com', timeout=30, verify=True)
httpx.post('https://gmail.com', timeout=30, verify=False)
httpx.put('https://gmail.com', timeout=30, verify=True)
httpx.put('https://gmail.com', timeout=30, verify=False)
httpx.patch('https://gmail.com', timeout=30, verify=True)
httpx.patch('https://gmail.com', timeout=30, verify=False)
httpx.delete('https://gmail.com', timeout=30, verify=True)
httpx.delete('https://gmail.com', timeout=30, verify=False)
httpx.stream('https://gmail.com', timeout=30, verify=True)
httpx.stream('https://gmail.com', timeout=30, verify=False)
httpx.Client(timeout=30)
httpx.Client(timeout=30, verify=False)
httpx.AsyncClient(timeout=30)
httpx.AsyncClient(timeout=30, verify=False)
4 changes: 2 additions & 2 deletions tests/functional/test_functional.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,8 @@ def test_requests_ssl_verify_disabled(self):
def test_requests_without_timeout(self):
"""Test for the `requests` library missing timeouts."""
expect = {
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 14, "HIGH": 0},
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 14, "MEDIUM": 0, "HIGH": 0},
"SEVERITY": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 36, "HIGH": 0},
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 36, "MEDIUM": 0, "HIGH": 0},
}
self.check_example("requests-missing-timeout.py", expect)

Expand Down
Loading