fix(security): Resolve SQL Injection vulnerability #264949956

Emilio Salvadorrequested to merge
duo/fix/264949956-sql-injection-2977131 into main

Security Fix: SQL Injection

Addresses: Vulnerability #264949956 detected by SAST

Vulnerability Details

  • ID: [vulnerability:264949956]
  • Type: SAST
  • Severity: High
  • Location: vulnerable_utils.py:360
  • Vulnerability Identifiers:
    • A1:2017 - Injection
    • A03:2021 - Injection
    • CWE-89
    • Bandit Test ID bandit.B608
    • python-lang-sqli-hardcoded-sql-expression-taint
    • SAST Rules ID - python_sql_rule-hardcoded-sql-expression

Source/Sink Analysis

Source (Data Origin):

  • Location: app.py:417
  • Description: User-controlled HTTP query parameter captured via request.args.get('airline', '') with no validation. The /api/flights/by-airline endpoint accepts arbitrary user input from the airline query parameter and passes it directly to the vulnerable function.

Data Flow:

  1. app.py:417 - User input captured from HTTP GET parameter via request.args.get('airline', '')
  2. app.py:426 - Untrusted input passed to search_flights_by_airline(airline) without any sanitization
  3. vulnerable_utils.py:342 - Function receives airline_name parameter
  4. vulnerable_utils.py:360 - airline_name interpolated into SQL query via f-string: f"SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = '{airline_name}'"
  5. vulnerable_utils.py:363 - cursor.execute(query) executes the constructed SQL string with embedded user input

Sink (Dangerous Operation):

  • Location: vulnerable_utils.py:360
  • Description: SQL query construction using Python f-string interpolation. The code uses f"SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = '{airline_name}'" which directly embeds user input into the SQL string. The subsequent cursor.execute(query) at line 363 executes this dynamically constructed query.

Root Cause: The code uses Python f-string formatting (f"...WHERE airline = '{airline_name}'") to construct SQL queries. F-strings perform simple string interpolation with ZERO escaping or validation. The sqlite3 cursor.execute() method supports parameterized queries using ? placeholders, which would treat parameters as data only. However, the code bypasses this protection by pre-constructing the entire query string. The single quotes around {airline_name} create a SQL string literal that can be broken out of by including a single quote in the user input, allowing injection of arbitrary SQL syntax.

Attack Vector:

Basic Authentication Bypass:

GET /api/flights/by-airline?airline=United' OR '1'='1
  • airline_name becomes "United' OR '1'='1"
  • Query becomes: SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = 'United' OR '1'='1'
  • Returns ALL flights (OR condition always true)

Data Exfiltration:

GET /api/flights/by-airline?airline=United' UNION SELECT id, name, email, ssn, credit_card FROM users WHERE '1'='1
  • Exfiltrates sensitive user data from users table by appending a UNION query

Data Destruction:

GET /api/flights/by-airline?airline=United'; DROP TABLE flights; --
  • Could delete the entire flights table

Remediation Approach

Strategy: Replace f-string interpolation with parameterized queries using SQLite's native parameter substitution mechanism (? placeholders)

Security Pattern Applied: Parameterized queries (prepared statements) - the industry-standard defense against SQL injection. The database driver treats parameters as data values only, never as executable SQL code.

Agent Analysis

I strongly agree with the automated fix. The analysis correctly identifies:

  1. Accurate vulnerability identification: The f-string interpolation at line 360 is a textbook SQL injection vulnerability with complete data flow tracing from HTTP input to SQL execution.

  2. Correct remediation strategy: Parameterized queries are the ONLY complete defense against SQL injection. The fix properly uses SQLite's ? placeholder mechanism and passes parameters as a tuple to cursor.execute().

  3. Surgical scope: The fix is appropriately limited to the single vulnerable function. No scope creep - only the reported vulnerability is addressed.

  4. Preservation of functionality: The parameterized query maintains identical behavior for legitimate inputs while completely preventing SQL injection.

  5. Complete elimination: By separating SQL syntax from data, the fix makes it impossible to inject SQL commands regardless of input content. Attack payloads like "United' OR '1'='1" will be treated as literal strings to match, not SQL syntax.

The fix follows OWASP SQL Injection Prevention guidelines and implements the exact solution recommended in the vulnerability description. This is a high-confidence, production-ready remediation.

Security Best Practices

This fix aligns with the following security standards and best practices:

OWASP SQL Injection Prevention Cheat Sheet:

CWE-89: Improper Neutralization of Special Elements used in an SQL Command:

OWASP Top 10 2021 - A03:2021 Injection:

Python DB-API 2.0 (PEP 249):

  • Reference: https://peps.python.org/pep-0249/
  • Standard: The execute() method should support parameter substitution using ? placeholders for SQLite
  • The fix correctly implements this standard: cursor.execute(query, (airline_name,))

SANS/CWE Top 25 Most Dangerous Software Weaknesses:

  • SQL Injection consistently ranks in the top 10 most dangerous weaknesses
  • Parameterized queries are the universally recommended mitigation

NIST SP 800-53 Security Controls:

  • SI-10: Information Input Validation
  • The fix ensures SQL parameters are validated by the database driver's parameterization mechanism

Changes Made

Primary Changes (Direct Vulnerability Fix)

vulnerable_utils.py (lines 342-368)

  • Change: Replaced f-string SQL query construction with parameterized query using ? placeholder
    • Line 360 (OLD): query = f"SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = '{airline_name}'"
    • Line 360 (NEW): query = "SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = ?"
    • Line 363 (OLD): cursor.execute(query)
    • Line 363 (NEW): cursor.execute(query, (airline_name,))
  • Purpose: Direct fix for the reported SQL injection vulnerability. Eliminates f-string interpolation and implements parameterized queries to ensure user input is treated as data only, never as SQL syntax.

Security Improvement

Before (Vulnerable State):

Line 360:

query = f"SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = '{airline_name}'"

Why vulnerable: F-string interpolation directly embeds user input into the SQL string. When airline_name contains SQL metacharacters like single quotes, they are interpreted as SQL syntax rather than data.

Example attack:

  • Input: airline_name = "United' OR '1'='1"
  • Resulting query: SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = 'United' OR '1'='1'
  • Impact: The OR '1'='1' condition is always true, returning ALL flights regardless of airline (100% attack success rate)

After (Secured State):

Line 360:

query = "SELECT flight_id, origin, destination, airline, price FROM flights WHERE airline = ?"

Line 363:

cursor.execute(query, (airline_name,))

Why secure: The ? placeholder separates SQL syntax from data. User input is passed as a parameter tuple (airline_name,) to cursor.execute(). The sqlite3 driver automatically escapes and quotes the parameter value, treating it as a literal string only.

Same attack attempt:

  • Input: airline_name = "United' OR '1'='1"
  • The sqlite3 driver treats this as a literal string to match
  • Effective query logic: Search for airline name literally equal to "United' OR '1'='1"
  • Impact: No airline has this exact name, so zero results returned. The SQL injection syntax is neutralized (0% attack success rate)

Impact:

The parameterized query completely eliminates SQL injection by ensuring user input is NEVER interpreted as SQL syntax. The mechanism works as follows:

  1. Syntax separation: The SQL query structure (SELECT ... WHERE airline = ?) is defined separately from data values
  2. Driver-level protection: The sqlite3 driver receives the query template and parameters separately
  3. Automatic escaping: The driver handles all necessary escaping and quoting of the parameter value
  4. Type safety: Parameters are treated as their declared type (string), making it impossible to inject SQL commands

This fix transforms the attack surface from "arbitrary SQL execution" to "string matching only," making SQL injection attacks impossible regardless of input content.

Verification

  • Vulnerability eliminated: ✅ true - F-string interpolation replaced with parameterized queries
  • Syntax valid: ✅ true - Code follows Python DB-API 2.0 standard for SQLite parameterization
  • Functionality preserved: ✅ true - Legitimate airline searches return identical results

Confidence Level: high

The SQL injection vulnerability has been completely eliminated by replacing f-string interpolation with parameterized queries. The fix changes line 360 from f"SELECT ... WHERE airline = '{airline_name}'" to "SELECT ... WHERE airline = ?" and line 363 from cursor.execute(query) to cursor.execute(query, (airline_name,)). This ensures the sqlite3 driver treats airline_name as a data value only, never as SQL syntax. The function maintains identical behavior for legitimate inputs while completely preventing SQL injection attacks. No other code changes are required as this is a self-contained function with a single SQL query.

Generated by GitLab Duo Workflow (ID: 2977131)

Merge request reports