Skip to content

Commit

Permalink
Avoid flagging HTTP and HTTPS literals in urllib-open
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Oct 18, 2023
1 parent dda4ced commit b90243b
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 5 deletions.
19 changes: 19 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import urllib

urllib.urlopen(url='http://www.google.com')
urllib.urlopen(url='http://www.google.com', **kwargs)
urllib.urlopen('http://www.google.com')
urllib.urlopen('file:///foo/bar/baz')
urllib.urlopen(url)

urllib.Request(url='http://www.google.com', **kwargs)
urllib.Request(url='http://www.google.com')
urllib.Request('http://www.google.com')
urllib.Request('file:///foo/bar/baz')
urllib.Request(url)

urllib.URLopener().open(fullurl='http://www.google.com', **kwargs)
urllib.URLopener().open(fullurl='http://www.google.com')
urllib.URLopener().open('http://www.google.com')
urllib.URLopener().open('file:///foo/bar/baz')
urllib.URLopener().open(url)
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_bandit/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ mod tests {
#[test_case(Rule::SubprocessWithoutShellEqualsTrue, Path::new("S603.py"))]
#[test_case(Rule::SuspiciousPickleUsage, Path::new("S301.py"))]
#[test_case(Rule::SuspiciousEvalUsage, Path::new("S307.py"))]
#[test_case(Rule::SuspiciousURLOpenUsage, Path::new("S310.py"))]
#[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))]
#[test_case(Rule::TryExceptContinue, Path::new("S112.py"))]
#[test_case(Rule::TryExceptPass, Path::new("S110.py"))]
Expand Down
23 changes: 18 additions & 5 deletions crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
//! Check for calls to suspicious functions, or calls into suspicious modules.
//!
//! See: <https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html>
use ruff_python_ast::ExprCall;

use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, ExprCall};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -851,9 +850,23 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
// MarkSafe
["django", "utils", "safestring", "mark_safe"] => Some(SuspiciousMarkSafeUsage.into()),
// URLOpen
["urllib", "urlopen" | "urlretrieve" | "URLopener" | "FancyURLopener" | "Request"] |
["urllib", "request", "urlopen" | "urlretrieve" | "URLopener" | "FancyURLopener"] |
["six", "moves", "urllib", "request", "urlopen" | "urlretrieve" | "URLopener" | "FancyURLopener"] => Some(SuspiciousURLOpenUsage.into()),
["urllib", "urlopen" | "urlretrieve" | "Request"] |
["urllib", "request", "urlopen" | "urlretrieve"] |
["six", "moves", "urllib", "request", "urlopen" | "urlretrieve"] => {
// If the `url` argument is a string literal, allow `http` and `https` schemes.
if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
if let Some(Expr::Constant(ast::ExprConstant { value: ast::Constant::Str(url), .. })) = &call.arguments.find_argument("url", 0) {
let url = url.trim_start();
if url.starts_with("http://") || url.starts_with("https://") {
return None;
}
}
}
Some(SuspiciousURLOpenUsage.into())
},
["urllib", "URLopener" | "FancyURLopener"] |
["urllib", "request", "URLopener" | "FancyURLopener"] |
["six", "moves", "urllib", "request", "URLopener" | "FancyURLopener"] => Some(SuspiciousURLOpenUsage.into()),
// NonCryptographicRandom
["random", "random" | "randrange" | "randint" | "choice" | "choices" | "uniform" | "triangular"] => Some(SuspiciousNonCryptographicRandomUsage.into()),
// UnverifiedContext
Expand Down
107 changes: 107 additions & 0 deletions ...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S310_S310.py.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S310.py:4:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
3 | urllib.urlopen(url='http://www.google.com')
4 | urllib.urlopen(url='http://www.google.com', **kwargs)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
5 | urllib.urlopen('http://www.google.com')
6 | urllib.urlopen('file:///foo/bar/baz')
|

S310.py:6:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
4 | urllib.urlopen(url='http://www.google.com', **kwargs)
5 | urllib.urlopen('http://www.google.com')
6 | urllib.urlopen('file:///foo/bar/baz')
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
7 | urllib.urlopen(url)
|

S310.py:7:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
5 | urllib.urlopen('http://www.google.com')
6 | urllib.urlopen('file:///foo/bar/baz')
7 | urllib.urlopen(url)
| ^^^^^^^^^^^^^^^^^^^ S310
8 |
9 | urllib.Request(url='http://www.google.com', **kwargs)
|

S310.py:9:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
7 | urllib.urlopen(url)
8 |
9 | urllib.Request(url='http://www.google.com', **kwargs)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
10 | urllib.Request(url='http://www.google.com')
11 | urllib.Request('http://www.google.com')
|

S310.py:12:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
10 | urllib.Request(url='http://www.google.com')
11 | urllib.Request('http://www.google.com')
12 | urllib.Request('file:///foo/bar/baz')
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
13 | urllib.Request(url)
|

S310.py:13:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
11 | urllib.Request('http://www.google.com')
12 | urllib.Request('file:///foo/bar/baz')
13 | urllib.Request(url)
| ^^^^^^^^^^^^^^^^^^^ S310
14 |
15 | urllib.URLopener().open(fullurl='http://www.google.com', **kwargs)
|

S310.py:15:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
13 | urllib.Request(url)
14 |
15 | urllib.URLopener().open(fullurl='http://www.google.com', **kwargs)
| ^^^^^^^^^^^^^^^^^^ S310
16 | urllib.URLopener().open(fullurl='http://www.google.com')
17 | urllib.URLopener().open('http://www.google.com')
|

S310.py:16:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
15 | urllib.URLopener().open(fullurl='http://www.google.com', **kwargs)
16 | urllib.URLopener().open(fullurl='http://www.google.com')
| ^^^^^^^^^^^^^^^^^^ S310
17 | urllib.URLopener().open('http://www.google.com')
18 | urllib.URLopener().open('file:///foo/bar/baz')
|

S310.py:17:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
15 | urllib.URLopener().open(fullurl='http://www.google.com', **kwargs)
16 | urllib.URLopener().open(fullurl='http://www.google.com')
17 | urllib.URLopener().open('http://www.google.com')
| ^^^^^^^^^^^^^^^^^^ S310
18 | urllib.URLopener().open('file:///foo/bar/baz')
19 | urllib.URLopener().open(url)
|

S310.py:18:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
16 | urllib.URLopener().open(fullurl='http://www.google.com')
17 | urllib.URLopener().open('http://www.google.com')
18 | urllib.URLopener().open('file:///foo/bar/baz')
| ^^^^^^^^^^^^^^^^^^ S310
19 | urllib.URLopener().open(url)
|

S310.py:19:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
|
17 | urllib.URLopener().open('http://www.google.com')
18 | urllib.URLopener().open('file:///foo/bar/baz')
19 | urllib.URLopener().open(url)
| ^^^^^^^^^^^^^^^^^^ S310
|


0 comments on commit b90243b

Please sign in to comment.