Skip to content

Resolve vulnerability: Improper neutralization of special elements used in an SQL Command ('SQL Injection')

David Pisekrequested to merge
remediate/improper-neutralization-of-special-elements-used-in-an-sql-comm-D20250619T045902 into main

MR created from vulnerability: Improper neutralization of special elements used in an SQL Command ('SQL Injection')

AI GENERATED FIX

The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. Use this feature with caution. Before you run a pipeline or apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.

The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context.

Please see our documentation for more information about this feature.

Description:

Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as SQLAlchemy which will protect your queries.

SQL Injections are a critical type of vulnerability that can lead to data or system compromise. By dynamically generating SQL query strings, user input may be able to influence the logic of an SQL statement. This could lead to an malicious parties accessing information they should not have access to, or in some circumstances, being able to execute OS functionality or code.

Replace all dynamically generated SQL queries with parameterized queries. In situations where dynamic queries must be created, never use direct user input, but instead use a map or dictionary of valid values and resolve them using a user supplied key.

For example, some database drivers do not allow parameterized queries for > or < comparison operators. In these cases, do not use a user supplied > or < value, but rather have the user supply a gt or lt value. The alphabetical values are then used to look up the > and < values to be used in the construction of the dynamic query. The same goes for other queries where column or table names are required but cannot be parameterized. Data that is possible user-controlled from a python request is passed to execute() function. To remediate this issue, use SQLAlchemy statements which are built with query parameterization and therefore not vulnerable to sql injection.

If for some reason this is not feasible, ensure calls including user-supplied data pass it in to the params parameter of the execute() method. Below is an example using execute(), passing in user-supplied data as params. This will treat the query as a parameterized query and params as strictly data, preventing any possibility of SQL Injection.

name = request.args.get('name')
req = text('SELECT * FROM student WHERE firstname = :x')
result = db.session.execute(req, {"x":name})

For more information on QuerySets see:

Analysis:

The vulnerability report identifies a SQL Injection vulnerability (CWE-89) in the code. Looking at the source code, I can see two instances where SQL queries are constructed using string formatting:

  1. In the /login route:
query = f"SELECT * FROM users WHERE username = '{username}' AND password = '{password}'"
  1. In the /user/<int:user_id> route:
query = f"SELECT * FROM users WHERE id = {user_id}"

Both of these are clear examples of SQL injection vulnerabilities. In the first case, user-supplied values for username and password are directly interpolated into the SQL query string. This allows an attacker to inject malicious SQL code. For example, if a user provides ' OR '1'='1 as the username and password, the resulting query would be:

SELECT * FROM users WHERE username = '' OR '1'='1' AND password = '' OR '1'='1'

This would bypass authentication by making the WHERE clause always evaluate to true.

In the second case, while user_id is specified as an integer in the route definition, it's still directly interpolated into the query string, which is unsafe.

The vulnerability report correctly identifies these issues and recommends using parameterized queries instead of string interpolation to construct SQL queries.

Summary:

SQL Injection Vulnerability Analysis and Fix

Vulnerability

The code contained SQL Injection vulnerabilities (CWE-89) in two locations where user input was directly interpolated into SQL query strings using f-strings. This allows attackers to inject malicious SQL code that could:

  • Bypass authentication
  • Access unauthorized data
  • Modify database contents
  • Potentially execute commands on the database server

Fix Implementation

I replaced the vulnerable string interpolation with parameterized queries:

  1. In the /login route:
# Before:
query = f"SELECT * FROM users WHERE username = '{username}' AND password = '{password}'"
cursor.execute(query)

# After:
query = "SELECT * FROM users WHERE username = %s AND password = %s"
cursor.execute(query, (username, password))
  1. In the /user/<int:user_id> route:
# Before:
query = f"SELECT * FROM users WHERE id = {user_id}"
cursor.execute(query)

# After:
query = "SELECT * FROM users WHERE id = %s"
cursor.execute(query, (user_id,))

Security Improvement

The fix uses the MySQL connector's parameterized query feature, which:

  • Separates SQL code from data
  • Properly escapes special characters in user input
  • Prevents SQL injection attacks by treating user input strictly as data, not executable code
  • Maintains the original functionality while eliminating the security vulnerability

This approach follows the OWASP recommendation to use parameterized queries instead of dynamically constructed SQL strings.

Identifiers:

  • A1:2017 - Injection
  • A03:2021 - Injection
  • CWE-89
  • python_flask_rule-flask-tainted-sql-string

Merge request reports