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.

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

Implement bandit's 'hardcoded-sql-expressions' S608 #2698

Merged
merged 5 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Add docs
  • Loading branch information
charliermarsh committed Feb 10, 2023
commit 57193ef17c08a14af9f949e22f52f2ba9ebc69ed
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/) on PyPI.
| S506 | unsafe-yaml-load | Probable use of unsafe loader `{name}` with `yaml.load`. Allows instantiation of arbitrary objects. Consider `yaml.safe_load`. | |
| S508 | snmp-insecure-version | The use of SNMPv1 and SNMPv2 is insecure. Use SNMPv3 if able. | |
| S509 | snmp-weak-cryptography | You should not use SNMPv3 without encryption. `noAuthNoPriv` & `authNoPriv` is insecure. | |
| S608 | hardcoded-sql-expression | Possible SQL injection vector through string-based query construction: "{}" | |
| S608 | [hardcoded-sql-expression](https://github.com/charliermarsh/ruff/blob/main/docs/rules/hardcoded-sql-expression.md) | Possible SQL injection vector through string-based query construction: "{}" | |
| S612 | logging-config-insecure-listen | Use of insecure `logging.config.listen` detected | |
| S701 | jinja2-autoescape-false | Using jinja2 templates with `autoescape=False` is dangerous and can lead to XSS. Ensure `autoescape=True` or use the `select_autoescape` function. | |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@ static SQL_REGEX: Lazy<Regex> = Lazy::new(|| {
});

define_violation!(
/// ### What it does
/// Checks for strings that resemble SQL statements involved in some form
/// string building operation.
///
/// ### Why is this bad?
/// SQL injection is a common attack vector for web applications. Unless care
/// is taken to sanitize and control the input data when building such
/// SQL statement strings, an injection attack becomes possible.
Comment on lines +24 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

This section implies that sanitizing inputs is alright ... when actually directly interpolating user input into SQL queries should always be avoided (no matter if sanitized or not).

You should instead use "parameter binding" ... also called "server-side binding", where the parameters are sent separately from the query to the server, see e.g. psycopg2 server-side binding.

So I think it would make sense to update this section accordingly.

///
/// ### Example
/// ```python
/// query = "DELETE FROM foo WHERE id = '%s'" % identifier
/// ```
pub struct HardcodedSQLExpression {
pub string: String,
}
Expand Down
17 changes: 17 additions & 0 deletions docs/rules/hardcoded-sql-expression.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# hardcoded-sql-expression (S608)

Derived from the **flake8-bandit** linter.

### What it does
Checks for strings that resemble SQL statements involved in some form
string building operation.

### Why is this bad?
SQL injection is a common attack vector for web applications. Unless care
is taken to sanitize and control the input data when building such
SQL statement strings, an injection attack becomes possible.

### Example
```python
query = "DELETE FROM foo WHERE id = '%s'" % identifier
```