Nutzung von f-Strings in SQL-Stratements ohne Gefahr der SQL-Injection

Installation und Anwendung von Datenbankschnittstellen wie SQLite, PostgreSQL, MariaDB/MySQL, der DB-API 2.0 und sonstigen Datenbanksystemen.
Antworten
Maschinenhans
User
Beiträge: 20
Registriert: Donnerstag 14. Juli 2011, 12:23

Hallo Forum,

in SQL statements (ich werde mich hier vornehmlich auf SQLite3 beziehen) soll man gemäß Python Doku ja die Substitutionen per "?" oder ":name" vornehmen. Wegen SQL-Injections. Das ist auch völlig in Ordnung.

Meine Frage ist: Ist das auch so, wenn ich die query erst als Liste anlege und in der execute Funktion den String per ' '.join(Liste) verkette? Zur Verdeutlichung ein kleines Minimalbeispiel:

Code: Alles auswählen

#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import sqlite3

def main():
    # Erst mal eine kleine Datenbank bauen
    connection = sqlite3.connect(':memory:')
    cursor = connection.cursor()

    cursor.execute('''
        CREATE TABLE test (
	        id	 INTEGER PRIMARY KEY NOT NULL UNIQUE,
	        nutzlast TEXT NOT NULL
        ); 
        ''')

    inhalt = [(1, 'Eintrag 5'),
              (2, 'Eintrag 4'),
              (3, 'Eintrag 3'),
              (4, 'Eintrag 2'),
              (5, 'Eintrag 1')]

    # So soll es ja sein
    cursor.executemany('INSERT INTO test VALUES (?, ?)', inhalt) 

    connection.commit()

    # Jetzt zur Frage:
    # Hat das folgende Konstrukt auch die benannte Sicherheitslücke? Die
    # Ersetzung der Werte im f-String passiert doch schon vor der Substitution
    # in der Abfrage während dem append(). Damit wäre die Gefahr der SQL-
    # Injection doch vom Tisch, oder?
    field = 'id'
    #field = 'nutzlast'
    
    query = ['SELECT * FROM test']
    query.append(f'ORDER BY {field}')

    result = cursor.execute(' '.join(query))
    rows = cursor.fetchall()

    for row in rows:
        print(f'{row[0]:2} {row[1]}')


if __name__ == '__main__':
    main()

# vim:tw=79:nowrap
Vielen Dank schon mal fürs Ansehen.

Gruß
Maschinenhans
Alles wird gut.
narpfel
User
Beiträge: 643
Registriert: Freitag 20. Oktober 2017, 16:10

Was ist, wenn `field` den Wert `"id; DROP TABLE test; --"` hat?
Maschinenhans
User
Beiträge: 20
Registriert: Donnerstag 14. Juli 2011, 12:23

Dann passiert das:

Code: Alles auswählen

Traceback (most recent call last):
  File "/path/to/test.py", line 49, in <module>
    main()
  File "/path/to/test.py", line 41, in main
    result = cursor.execute(' '.join(query))
sqlite3.Warning: You can only execute one statement at a time.
Ich finde, dass macht Mut.
Alles wird gut.
narpfel
User
Beiträge: 643
Registriert: Freitag 20. Oktober 2017, 16:10

Und wenn es `"id LIMIT 0"` ist?
Maschinenhans
User
Beiträge: 20
Registriert: Donnerstag 14. Juli 2011, 12:23

Dann wird kein Fehler, aber auch keine Zeile ausgegeben.

Dann ist es aber auch ein valides SQL-Statement, was -natürlich mit einem anderen Limit- so gewollt sein könnte.
Alles wird gut.
narpfel
User
Beiträge: 643
Registriert: Freitag 20. Oktober 2017, 16:10

Maschinenhans hat geschrieben: Dienstag 29. Juni 2021, 09:20 Dann ist es aber auch ein valides SQL-Statement
Ja, das ist genau SQL Injection. Mit einem ungültigen SQL-Statement würde das ja auch wenig Sinn machen?

Wenn es möglich ist, dass ein Nutzer "id LIMIT 0" angibt und das als SQL interpretiert wird und nicht als String im SQL-Statement, dann hast du ein potentielles Problem. Du musst nur jemanden finden, der besser SQL kann als ich, um das noch mehr kaputt zu machen. ;-)
Sirius3
User
Beiträge: 17711
Registriert: Sonntag 21. Oktober 2012, 17:20

Man kann in begrenztem Maße Queryies zusammenbauen, dann muß aber auch der Input streng validiert werden.
Für solch komplizierten Konstrukte wäre es aber besser auf ein Paket wie SQLAlchemy umzusteigen.
*-SELECTS sind schlecht, gib immer die konkreten Felder an.
Maschinenhans
User
Beiträge: 20
Registriert: Donnerstag 14. Juli 2011, 12:23

narpfel hat geschrieben: Dienstag 29. Juni 2021, 09:30 ...

Ja, das ist genau SQL Injection. Mit einem ungültigen SQL-Statement würde das ja auch wenig Sinn machen?

...
Da hast du recht :roll: :oops: .

Ein Umschreiben in:

Code: Alles auswählen

...
result = cursor.execute('SELECT * FROM test ORDER BY ?', ('id LIMIT 0',))
...
Ergibt die Ausgabe aller Zeilen. Hmmm ...

Also kann ich doch nicht einfach per ' '.join(zeuch) alles zusammenfegen und übergeben. Schade.

Danke für die Hilfe und deine Zeit.
Alles wird gut.
Benutzeravatar
__blackjack__
User
Beiträge: 13004
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@Maschinenhans: NOT NULL und UNIQUE sind überflüssig wenn die Spalte bereits als PRIMARY KEY deklariert ist.

Das ganze mit SQLAlchemy (ohne ORM):

Code: Alles auswählen

#!/usr/bin/env python3
from sqlalchemy import Column, Integer, MetaData, Table, Text, create_engine


def main():
    engine = create_engine("sqlite://")
    meta_data = MetaData(engine)
    test_table = Table(
        "test",
        meta_data,
        Column("id", Integer, primary_key=True),
        Column("nutzlast", Text, nullable=False),
    )
    meta_data.create_all()
    engine.execute(
        test_table.insert(),
        {"id": 1, "nutzlast": "Eintrag 5"},
        {"id": 2, "nutzlast": "Eintrag 4"},
        {"id": 3, "nutzlast": "Eintrag 3"},
        {"id": 4, "nutzlast": "Eintrag 2"},
        {"id": 5, "nutzlast": "Eintrag 1"},
    )

    field = "id"
    # field = "nutzlast"
    query = test_table.select().order_by(test_table.c[field])
    for row in engine.execute(query):
        print(f"{row.id:2} {row.nutzlast}")


if __name__ == "__main__":
    main()

# vim:tw=79:nowrap
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
LukeNukem
User
Beiträge: 232
Registriert: Mittwoch 19. Mai 2021, 03:40

Maschinenhans hat geschrieben: Dienstag 29. Juni 2021, 09:41 Also kann ich doch nicht einfach per ' '.join(zeuch) alles zusammenfegen und übergeben. Schade.
Der Grund, warum die DB-API 2.0 (PEP249) einen "paramstyle" festlegt, ist, daß der Datenbanktreiber die Parameter dann anhand der dafür vorgesehenen Spaltentypen korrekt formatieren und dabei auch Fehler werfen kann, wenn Du etwas übergibst, das nicht in die Spaßte paßt. Da haben sich die Entwickler der API und der Treiber schon Gedanken drüber gemacht, und ich würde vermuten, daß die meisten dieser Entwickler wesentlich mehr über SQL und die damit zusammenhängenden Gefahren wissen als wir alle hier zusammen, mich selbst ausdrücklich eingeschlossen. Alles, was Du mit F-Strings, .format(), %() oder String Concatenation machst, ist inhärent anfällig für SQL-Injections -- und Deine Variablen korrekt zu validieren, ist eine hohe Kunst für sich und womöglich viel Arbeit -- die Du Dir zumindest teilweise ganz einfach ersparen kannst, wenn Du es richtig machst und den "paramstyle" benutzt.

Und wenn ich Dir angelegentlich einen guten Rat geben darf: bitte probier' so etwas mit einem richtigen RDBMS wie PostgreSQL oder Ähnlichem aus, nicht mit einem "SQL-ähnlichen Festplattentreiber" (Joey Celko) wie SqLite. SqLite unterstützt nur ein Subset der einschlägigen Standards und ist, Pardon, vielleicht für Spielkram gut, aber leider nicht für ernsthafte Tests -- schon gar nicht, wenn es um sicherheitsrelevante Funktionen geht.
Maschinenhans
User
Beiträge: 20
Registriert: Donnerstag 14. Juli 2011, 12:23

Hallo LukeNukem,

vielen Dank für die Erklärung.

Dass das in diesem "paramstyle" festgelegt ist, hat ganz sicher einen guten Grund. Auch bin ich der festen Überzeugung, dass die ganze Verkettung von Strings auf sichere Art implementiert wäre, wenn das in einem technisch und vom Umfang her sinnvollen Rahmen, machbar wäre.
LukeNukem hat geschrieben: Donnerstag 1. Juli 2021, 19:55 ...
Und wenn ich Dir angelegentlich einen guten Rat geben darf: bitte probier' so etwas mit einem richtigen RDBMS wie PostgreSQL oder Ähnlichem aus, nicht mit einem "SQL-ähnlichen Festplattentreiber" (Joey Celko) wie SqLite. SqLite unterstützt nur ein Subset der einschlägigen Standards und ist, Pardon, vielleicht für Spielkram gut, aber leider nicht für ernsthafte Tests -- schon gar nicht, wenn es um sicherheitsrelevante Funktionen geht.
Ich bin Amateur und benutze das alles nur als Spielkram (hier: ein CLI-Programm zum Verwalten von Bookmarks, wie buku). Über den sehr abgespekten Funktionsumfang von SQLite bin ich mir im Klaren.

Meine Motivation ist allerdings, auch in kleinen Skripten und Programmen, die auch nicht zur Veröffentlichung vorgesehen sind, handwerklich saubere und gute Arbeit zu leisten. Dazu gehört für mich auch, die sicherheitsrelevanten Fallstricke zu behandeln, die ich kenne. Ausserdem möchte ich mich weiterentwickeln und suche nach neuen Lösungen, die meinen Horizont erweitern.

Dass das alles etwas infantil aussehen mag, könnte daran liegen, dass ich mich beruflich in einem gänzlich anderen Bereich bewege und den großen Komplex Programmieren/Softwareentwicklung autodidaktisch erforsche.

An der Entwicklung von sicherheitsrelevanter oder Unternehmenskritischer Software bin ich nicht beteiligt.

Gruß
Maschinenhans
Alles wird gut.
Antworten