Skip to content

Commit 4ea4bbb

Browse files
DataEnggNerdSanthosh SolomonMichaReiser
authored
[flake8-bandit] Detect patterns from multi line SQL statements (S608) (#13574)
Co-authored-by: Santhosh Solomon <[email protected]> Co-authored-by: Micha Reiser <[email protected]>
1 parent ed4a0b3 commit 4ea4bbb

File tree

3 files changed

+154
-29
lines changed

3 files changed

+154
-29
lines changed

‎crates/ruff_linter/resources/test/fixtures/flake8_bandit/S608.py

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,18 +95,62 @@ def query41():
9595
cursor.executemany('SELECT * FROM table WHERE id = %s', [var, var2])
9696

9797
# # INSERT without INTO (e.g. MySQL and derivatives)
98-
query = "INSERT table VALUES (%s)" % (var,)
98+
query46 = "INSERT table VALUES (%s)" % (var,)
9999

100100
# # REPLACE (e.g. MySQL and derivatives, SQLite)
101-
query = "REPLACE INTO table VALUES (%s)" % (var,)
102-
query = "REPLACE table VALUES (%s)" % (var,)
101+
query47 = "REPLACE INTO table VALUES (%s)" % (var,)
102+
query48 = "REPLACE table VALUES (%s)" % (var,)
103103

104-
query = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
104+
query49 = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
105105

106106
# # pass
107107
["select colA from tableA"] + ["select colB from tableB"]
108108
"SELECT * FROM " + (["table1"] if x > 0 else ["table2"])
109109

110110
# # errors
111-
"SELECT * FROM " + ("table1" if x > 0 else "table2")
112-
"SELECT * FROM " + ("table1" if x > 0 else ["table2"])
111+
"SELECT * FROM " + ("table1" if x > 0 else "table2") # query50
112+
"SELECT * FROM " + ("table1" if x > 0 else ["table2"]) # query51
113+
114+
# test cases from #12044
115+
116+
def query52():
117+
return f"""
118+
SELECT {var}
119+
FROM bar
120+
"""
121+
122+
def query53():
123+
return f"""
124+
SELECT
125+
{var}
126+
FROM bar
127+
"""
128+
129+
def query54():
130+
return f"""
131+
SELECT {var}
132+
FROM
133+
bar
134+
"""
135+
136+
query55 = f"""SELECT * FROM
137+
{var}.table
138+
"""
139+
140+
query56 = f"""SELECT *
141+
FROM {var}.table
142+
"""
143+
144+
query57 = f"""
145+
SELECT *
146+
FROM {var}.table
147+
"""
148+
149+
query57 = f"""
150+
PRESELECT *
151+
FROM {var}.table
152+
"""
153+
154+
# to be handled seperately
155+
# query58 = f"SELECT\
156+
# * FROM {var}.table"

‎crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_sql_expression.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
use once_cell::sync::Lazy;
22
use regex::Regex;
3-
use ruff_python_ast::{self as ast, Expr, Operator};
4-
53
use ruff_diagnostics::{Diagnostic, Violation};
64
use ruff_macros::{derive_message_formats, violation};
75
use ruff_python_ast::str::raw_contents;
6+
use ruff_python_ast::{self as ast, Expr, Operator};
87
use ruff_source_file::Locator;
98
use ruff_text_size::Ranged;
109

1110
use crate::checkers::ast::Checker;
1211

1312
static SQL_REGEX: Lazy<Regex> = Lazy::new(|| {
14-
Regex::new(r"(?i)\b(select\s.+\sfrom\s|delete\s+from\s|(insert|replace)\s.+\svalues\s|update\s.+\sset\s)")
15-
.unwrap()
13+
Regex::new(r"(?i)\b(select\s+.*\s+from\s|delete\s+from\s|(insert|replace)\s+.*\s+values\s|update\s+.*\s+set\s)")
14+
.unwrap()
1615
});
1716

1817
/// ## What it does
@@ -88,6 +87,7 @@ pub(crate) fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) {
8887
};
8988
string.value.to_str().escape_default().to_string()
9089
}
90+
9191
// f"select * from table where val = {val}"
9292
Expr::FString(f_string) => concatenated_f_string(f_string, checker.locator()),
9393
_ => return,
@@ -113,9 +113,7 @@ pub(crate) fn hardcoded_sql_expression(checker: &mut Checker, expr: &Expr) {
113113
fn concatenated_f_string(expr: &ast::ExprFString, locator: &Locator) -> String {
114114
expr.value
115115
.iter()
116-
.filter_map(|part| {
117-
raw_contents(locator.slice(part)).map(|s| s.escape_default().to_string())
118-
})
116+
.filter_map(|part| raw_contents(locator.slice(part)))
119117
.collect()
120118
}
121119

‎crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S608_S608.py.snap

Lines changed: 99 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -452,45 +452,128 @@ S608.py:86:30: S608 Possible SQL injection vector through string-based query con
452452
88 | # # pass
453453
|
454454

455-
S608.py:98:9: S608 Possible SQL injection vector through string-based query construction
455+
S608.py:98:11: S608 Possible SQL injection vector through string-based query construction
456456
|
457457
97 | # # INSERT without INTO (e.g. MySQL and derivatives)
458-
98 | query = "INSERT table VALUES (%s)" % (var,)
459-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
458+
98 | query46 = "INSERT table VALUES (%s)" % (var,)
459+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
460460
99 |
461461
100 | # # REPLACE (e.g. MySQL and derivatives, SQLite)
462462
|
463463

464-
S608.py:101:9: S608 Possible SQL injection vector through string-based query construction
464+
S608.py:101:11: S608 Possible SQL injection vector through string-based query construction
465465
|
466466
100 | # # REPLACE (e.g. MySQL and derivatives, SQLite)
467-
101 | query = "REPLACE INTO table VALUES (%s)" % (var,)
468-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
469-
102 | query = "REPLACE table VALUES (%s)" % (var,)
467+
101 | query47 = "REPLACE INTO table VALUES (%s)" % (var,)
468+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
469+
102 | query48 = "REPLACE table VALUES (%s)" % (var,)
470470
|
471471

472-
S608.py:102:9: S608 Possible SQL injection vector through string-based query construction
472+
S608.py:102:11: S608 Possible SQL injection vector through string-based query construction
473473
|
474474
100 | # # REPLACE (e.g. MySQL and derivatives, SQLite)
475-
101 | query = "REPLACE INTO table VALUES (%s)" % (var,)
476-
102 | query = "REPLACE table VALUES (%s)" % (var,)
477-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
475+
101 | query47 = "REPLACE INTO table VALUES (%s)" % (var,)
476+
102 | query48 = "REPLACE table VALUES (%s)" % (var,)
477+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
478478
103 |
479-
104 | query = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
479+
104 | query49 = "Deselect something that is not SQL even though it has a ' from ' somewhere in %s." % "there"
480480
|
481481

482482
S608.py:111:1: S608 Possible SQL injection vector through string-based query construction
483483
|
484484
110 | # # errors
485-
111 | "SELECT * FROM " + ("table1" if x > 0 else "table2")
485+
111 | "SELECT * FROM " + ("table1" if x > 0 else "table2") # query50
486486
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
487-
112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"])
487+
112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"]) # query51
488488
|
489489

490490
S608.py:112:1: S608 Possible SQL injection vector through string-based query construction
491491
|
492492
110 | # # errors
493-
111 | "SELECT * FROM " + ("table1" if x > 0 else "table2")
494-
112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"])
493+
111 | "SELECT * FROM " + ("table1" if x > 0 else "table2") # query50
494+
112 | "SELECT * FROM " + ("table1" if x > 0 else ["table2"]) # query51
495495
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S608
496+
113 |
497+
114 | # test cases from #12044
498+
|
499+
500+
S608.py:117:12: S608 Possible SQL injection vector through string-based query construction
501+
|
502+
116 | def query52():
503+
117 | return f"""
504+
| ____________^
505+
118 | | SELECT {var}
506+
119 | | FROM bar
507+
120 | | """
508+
| |_______^ S608
509+
121 |
510+
122 | def query53():
511+
|
512+
513+
S608.py:123:12: S608 Possible SQL injection vector through string-based query construction
514+
|
515+
122 | def query53():
516+
123 | return f"""
517+
| ____________^
518+
124 | | SELECT
519+
125 | | {var}
520+
126 | | FROM bar
521+
127 | | """
522+
| |_______^ S608
523+
128 |
524+
129 | def query54():
525+
|
526+
527+
S608.py:130:12: S608 Possible SQL injection vector through string-based query construction
528+
|
529+
129 | def query54():
530+
130 | return f"""
531+
| ____________^
532+
131 | | SELECT {var}
533+
132 | | FROM
534+
133 | | bar
535+
134 | | """
536+
| |_______^ S608
537+
135 |
538+
136 | query55 = f"""SELECT * FROM
539+
|
540+
541+
S608.py:136:11: S608 Possible SQL injection vector through string-based query construction
542+
|
543+
134 | """
544+
135 |
545+
136 | query55 = f"""SELECT * FROM
546+
| ___________^
547+
137 | | {var}.table
548+
138 | | """
549+
| |___^ S608
550+
139 |
551+
140 | query56 = f"""SELECT *
552+
|
553+
554+
S608.py:140:11: S608 Possible SQL injection vector through string-based query construction
555+
|
556+
138 | """
557+
139 |
558+
140 | query56 = f"""SELECT *
559+
| ___________^
560+
141 | | FROM {var}.table
561+
142 | | """
562+
| |___^ S608
563+
143 |
564+
144 | query57 = f"""
565+
|
566+
567+
S608.py:144:11: S608 Possible SQL injection vector through string-based query construction
568+
|
569+
142 | """
570+
143 |
571+
144 | query57 = f"""
572+
| ___________^
573+
145 | | SELECT *
574+
146 | | FROM {var}.table
575+
147 | | """
576+
| |___^ S608
577+
148 |
578+
149 | query57 = f"""
496579
|

0 commit comments

Comments
 (0)