Possible SQL injection vulnerability with f-strings

[983]

I would suggest to remove the line

query = f"INSERT INTO STATION VALUES (13, '{city}', '{state}', {latitude}, {longitude})"

since f-strings in SQL statements allow for SQL injection attacks.

Example:

import sqlite3

# create in-memory database
with sqlite3.connect(":memory:") as c:

    # create user table with two users
    c.execute('CREATE TABLE users (name TEXT, password TEXT)')
    c.execute('INSERT INTO users VALUES ("attacker", "Oz57SuEYL0kntSWV")')
    c.execute('INSERT INTO users VALUES ("victim", "A99JBWebpn1WIewx")')

    def change_password(name, new_password):
        # here's the problem
        c.execute(f'UPDATE users SET password = "{new_password}" WHERE name = "{name}"')
        c.commit()

    change_password('" or "1"="1', 'hacked password')

    rows = c.execute('SELECT * FROM USERS')

    for user, password in rows:
        print(f'{user}\'s password is: {password}')
    
    # now victim's password has been hacked by attacker

Instead, use the DB-API’s parameter substitution.
https://docs.python.org/3/library/sqlite3.html

[arogozhnikov]

I actually expect users to be aware of SQL injections, but I'm using such approach e.g. to generate python scripts. SQL was taken as a clearer example. Will add a note about this.

[983]

If all users were aware of SQL injections there wouldn't be any. But SQL injection attacks rank among the most common attacks on the internet, so we should not assume that everyone is familiar with them.

I saw you added the comment:

don't forget to escape arguments to prevent SQL injection attacks.

This is error-prone because programmers are humans and humans will forget to escape all arguments every single time.

It is advised to pass the parameters into the execute function instead because they will be escaped automatically:

# sqlite
c.execute("INSERT INTO users VALUES (? , ?)", (name, password))

# MySQL
c.execute("INSERT INTO users VALUES (%s, %s)", (name, password))

[aaossa]

Maybe a pointer to this issue and it's examples is enough. I know SQL injection is an important issue, but the intention of the example is to show a Python feature, not to teach how to write an SQL query in a safe way.

[acdha]

@aaossa I think that's a risk: we have a lot of past history showing that people will tend to follow the first way they learned how to do something — e.g. with PHP it was always possible to use bind variables safely or validate inputs but many, many people used string interpolation because they learned that first and it was easy.

To be honest, I'd be tempted to remove that example entirely to avoid opening this issue up.

[arogozhnikov]

Ok, I've deleted this example. Honestly, I see no possibility of an attack in the last version (but maybe I'm wrong)

query = f"INSERT INTO STATION VALUES (13, {city!r}, {state!r}, {latitude!r}, {longitude!r})"

[983]

Thanks, I appreciate the deletion.

Honestly, I see no possibility of an attack in the last version

Is this being used anywhere or can I show you how to exploit it?

[arogozhnikov]

Is this being used anywhere or can I show you how to exploit it?

Nope, that was my idea of simple escaping, I'm curious when it fails.

[983]

import sqlite3

with sqlite3.connect(":memory:") as c:
    c.execute('CREATE TABLE users (name TEXT, password TEXT, hobbies TEXT)')

    def add_user(name, password, hobbies):
        c.execute(f"INSERT INTO users VALUES ({name!r}, {password!r}, {hobbies!r})")

    add_user("victim", "victim's secret password", "biking, fishing")
    # steal victim's password and set as hobby
    add_user("attacker", "\"', (SELECT password FROM users)) -- ", "")
    
    for user, password, hobbies in c.execute('SELECT * FROM USERS'):
        print(f"{user}'s hobbies are: {hobbies}")
    # victim's hobbies are: biking, fishing
    # attacker's hobbies are: victim's secret password