Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[`flake8-bandit`] Add Rule for `S506` (unsafe use of yaml load) (#1664) 

See: https://github.com/charliermarsh/ruff/issues/1646.

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
Maksudul Haque 2023-01-06 00:35:01 +06:00 committed by GitHub
parent 5eb03d5e09
commit 2d23b1ae69
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 148 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
README.md
View File

@ -767,11 +767,12 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on
| S102 | ExecUsed | Use of `exec` detected | |
| S103 | BadFilePermissions | `os.chmod` setting a permissive mask `0o777` on file or directory | |
| S104 | HardcodedBindAllInterfaces | Possible binding to all interfaces | |
| S105 | HardcodedPasswordString | Possible hardcoded password: `"..."` | |
| S106 | HardcodedPasswordFuncArg | Possible hardcoded password: `"..."` | |
| S107 | HardcodedPasswordDefault | Possible hardcoded password: `"..."` | |
| S108 | HardcodedTempFile | Probable insecure usage of temp file/directory: `"..."` | |
| S324 | HashlibInsecureHashFunction | Probable use of insecure hash functions in hashlib: `"..."` | |
| S105 | HardcodedPasswordString | Possible hardcoded password: "..." | |
| S106 | HardcodedPasswordFuncArg | Possible hardcoded password: "..." | |
| S107 | HardcodedPasswordDefault | Possible hardcoded password: "..." | |
| S108 | HardcodedTempFile | Probable insecure usage of temporary file or directory: "..." | |
| S324 | HashlibInsecureHashFunction | Probable use of insecure hash functions in `hashlib`: "..." | |
| S506 | UnsafeYAMLLoad | Probable use of unsafe `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`. | |
### flake8-blind-except (BLE)

31
resources/test/fixtures/flake8_bandit/S506.py vendored Normal file
View File

@ -0,0 +1,31 @@
import json
import yaml
from yaml import CSafeLoader
from yaml import SafeLoader
from yaml import SafeLoader as NewSafeLoader
def test_yaml_load():
ystr = yaml.dump({"a": 1, "b": 2, "c": 3})
y = yaml.load(ystr)
yaml.dump(y)
try:
y = yaml.load(ystr, Loader=yaml.CSafeLoader)
except AttributeError:
# CSafeLoader only exists if you build yaml with LibYAML
y = yaml.load(ystr, Loader=yaml.SafeLoader)
def test_json_load():
# no issue should be found
j = json.load("{}")
yaml.load("{}", Loader=yaml.Loader)
# no issue should be found
yaml.load("{}", SafeLoader)
yaml.load("{}", yaml.SafeLoader)
yaml.load("{}", CSafeLoader)
yaml.load("{}", yaml.CSafeLoader)
yaml.load("{}", NewSafeLoader)

3
ruff.schema.json
View File

@ -930,6 +930,9 @@
"S3",
"S32",
"S324",
"S5",
"S50",
"S506",
"SIM",
"SIM1",
"SIM10",

11
src/checkers/ast.rs
View File

@ -1896,6 +1896,17 @@ where
self.add_check(check);
}
}
if self.settings.enabled.contains(&CheckCode::S506) {
if let Some(check) = flake8_bandit::checks::unsafe_yaml_load(
func,
args,
keywords,
&self.from_imports,
&self.import_aliases,
) {
self.add_check(check);
}
}
if self.settings.enabled.contains(&CheckCode::S106) {
self.add_checks(
flake8_bandit::checks::hardcoded_password_func_arg(keywords).into_iter(),

2
src/flake8_bandit/checks/mod.rs
View File

@ -9,6 +9,7 @@ pub use hardcoded_password_string::{
};
pub use hardcoded_tmp_directory::hardcoded_tmp_directory;
pub use hashlib_insecure_hash_functions::hashlib_insecure_hash_functions;
pub use unsafe_yaml_load::unsafe_yaml_load;
mod assert_used;
mod bad_file_permissions;
@ -19,3 +20,4 @@ mod hardcoded_password_func_arg;
mod hardcoded_password_string;
mod hardcoded_tmp_directory;
mod hashlib_insecure_hash_functions;
mod unsafe_yaml_load;

50
src/flake8_bandit/checks/unsafe_yaml_load.rs Normal file
View File

@ -0,0 +1,50 @@
use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_ast::{Expr, ExprKind, Keyword};
use crate::ast::helpers::{match_module_member, SimpleCallArgs};
use crate::ast::types::Range;
use crate::registry::{Check, CheckKind};
/// S506
pub fn unsafe_yaml_load(
func: &Expr,
args: &[Expr],
keywords: &[Keyword],
from_imports: &FxHashMap<&str, FxHashSet<&str>>,
import_aliases: &FxHashMap<&str, &str>,
) -> Option<Check> {
if match_module_member(func, "yaml", "load", from_imports, import_aliases) {
let call_args = SimpleCallArgs::new(args, keywords);
if let Some(loader_arg) = call_args.get_argument("Loader", Some(1)) {
if !match_module_member(
loader_arg,
"yaml",
"SafeLoader",
from_imports,
import_aliases,
) && !match_module_member(
loader_arg,
"yaml",
"CSafeLoader",
from_imports,
import_aliases,
) {
let loader = match &loader_arg.node {
ExprKind::Attribute { attr, .. } => Some(attr.to_string()),
ExprKind::Name { id, .. } => Some(id.to_string()),
_ => None,
};
return Some(Check::new(
CheckKind::UnsafeYAMLLoad(loader),
Range::from_located(loader_arg),
));
}
} else {
return Some(Check::new(
CheckKind::UnsafeYAMLLoad(None),
Range::from_located(func),
));
}
}
None
}

1
src/flake8_bandit/mod.rs
View File

@ -22,6 +22,7 @@ mod tests {
#[test_case(CheckCode::S107, Path::new("S107.py"); "S107")]
#[test_case(CheckCode::S108, Path::new("S108.py"); "S108")]
#[test_case(CheckCode::S324, Path::new("S324.py"); "S324")]
#[test_case(CheckCode::S506, Path::new("S506.py"); "S506")]
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
let checks = test_path(

25
src/flake8_bandit/snapshots/ruff__flake8_bandit__tests__S506_S506.py.snap Normal file
View File

@ -0,0 +1,25 @@
---
source: src/flake8_bandit/mod.rs
expression: checks
---
- kind:
UnsafeYAMLLoad: ~
location:
row: 10
column: 8
end_location:
row: 10
column: 17
fix: ~
parent: ~
- kind:
UnsafeYAMLLoad: Loader
location:
row: 24
column: 23
end_location:
row: 24
column: 34
fix: ~
parent: ~

25
src/registry.rs
View File

@ -335,6 +335,7 @@ pub enum CheckCode {
S107,
S108,
S324,
S506,
// flake8-boolean-trap
FBT001,
FBT002,
@ -1075,6 +1076,7 @@ pub enum CheckKind {
HardcodedPasswordDefault(String),
HardcodedTempFile(String),
HashlibInsecureHashFunction(String),
UnsafeYAMLLoad(Option<String>),
// mccabe
FunctionIsTooComplex(String, usize),
// flake8-boolean-trap
@ -1538,6 +1540,7 @@ impl CheckCode {
CheckCode::S107 => CheckKind::HardcodedPasswordDefault("...".to_string()),
CheckCode::S108 => CheckKind::HardcodedTempFile("...".to_string()),
CheckCode::S324 => CheckKind::HashlibInsecureHashFunction("...".to_string()),
CheckCode::S506 => CheckKind::UnsafeYAMLLoad(None),
// mccabe
CheckCode::C901 => CheckKind::FunctionIsTooComplex("...".to_string(), 10),
// flake8-boolean-trap
@ -1941,6 +1944,7 @@ impl CheckCode {
CheckCode::S107 => CheckCategory::Flake8Bandit,
CheckCode::S108 => CheckCategory::Flake8Bandit,
CheckCode::S324 => CheckCategory::Flake8Bandit,
CheckCode::S506 => CheckCategory::Flake8Bandit,
// flake8-simplify
CheckCode::SIM102 => CheckCategory::Flake8Simplify,
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
@ -2317,6 +2321,7 @@ impl CheckKind {
CheckKind::HardcodedPasswordDefault(..) => &CheckCode::S107,
CheckKind::HardcodedTempFile(..) => &CheckCode::S108,
CheckKind::HashlibInsecureHashFunction(..) => &CheckCode::S324,
CheckKind::UnsafeYAMLLoad(..) => &CheckCode::S506,
// mccabe
CheckKind::FunctionIsTooComplex(..) => &CheckCode::C901,
// flake8-boolean-trap
@ -3260,23 +3265,31 @@ impl CheckKind {
CheckKind::HardcodedPasswordString(string)
| CheckKind::HardcodedPasswordFuncArg(string)
| CheckKind::HardcodedPasswordDefault(string) => {
format!(
"Possible hardcoded password: `\"{}\"`",
string.escape_debug()
)
format!("Possible hardcoded password: \"{}\"", string.escape_debug())
}
CheckKind::HardcodedTempFile(string) => {
format!(
"Probable insecure usage of temp file/directory: `\"{}\"`",
"Probable insecure usage of temporary file or directory: \"{}\"",
string.escape_debug()
)
}
CheckKind::HashlibInsecureHashFunction(string) => {
format!(
"Probable use of insecure hash functions in hashlib: `\"{}\"`",
"Probable use of insecure hash functions in `hashlib`: \"{}\"",
string.escape_debug()
)
}
CheckKind::UnsafeYAMLLoad(loader) => match loader {
Some(name) => {
format!(
"Probable use of unsafe loader `{name}` with `yaml.load`. Allows \
instantiation of arbitrary objects. Consider `yaml.safe_load`."
)
}
None => "Probable use of unsafe `yaml.load`. Allows instantiation of arbitrary \
objects. Consider `yaml.safe_load`."
.to_string(),
},
// flake8-blind-except
CheckKind::BlindExcept(name) => format!("Do not catch blind exception: `{name}`"),
// mccabe