SQLight Sicherheit und Codeanalysen

DTF · 17. Januar 2024, 12:12

Haudruferzappeltnoch schrieb:

Anscheinend gibt es doch benannte Parameter

Ja die gibt es, siehe Source ^, da habe ich das gezeigt.

Amelie · 18. Januar 2024, 07:31

@DTF

Danke, aber ich habe das Problem ja nicht bei einer "Select" Abfrage sondern hier:

VB.NET-Quellcode

Using command As New SQLiteCommand(query, connection)

Wie ich es auch drehe und wende, es scheitert immer an dieser Zeile.

VB.NET-Quellcode

Public Sub LoadTableColumns(tableName As String)
Con2DataBase()
Dim columnList As New List(Of String)()
Dim query As String = $"PRAGMA table_info({QuoteIdentifier(tableName)})"
Debug.WriteLine($"Class: {query}")
Using command As New SQLiteCommand(query, connection)
Using reader As SQLiteDataReader = command.ExecuteReader()
While reader.Read()
Dim columnName As String = reader("name").ToString()
columnList.Add(columnName)
End While
End Using
End Using
CloseConnection()
RaiseEvent ColumnsLoaded(columnList)
End Sub
Private Function QuoteIdentifier(identifier As String) As String
' Funktion zum Sicheren Umgeben von Tabellen-/Spaltennamen mit Anführungszeichen
Return $"`{identifier.Replace("`", "``")}`"
End Function

Auf dem Form wo die Tabelle ausgwählt wird.

VB.NET-Quellcode

Private Sub ListBoxTables_SelectedIndexChanged(sender As Object, e As EventArgs) Handles ListBoxTables.SelectedIndexChanged
If ListBoxTables.SelectedItem IsNot Nothing Then
DBConnection.SelectedTable = ListBoxTables.SelectedItem.ToString()
Debug.WriteLine($"Form: {DBConnection.SelectedTable}")
' Weitere Verarbeitung oder Aufruf anderer Methoden
DBConnection.LoadTableColumns(DBConnection.SelectedTable)
End If
End Sub

DTF · 18. Januar 2024, 08:45

Also wo liegt denn jetzt genau das Problem? Nur die TableInfo zu bekommen? Tablenamen funktionieren ja nicht NamedParam. Wenn es dir nur um die TableInfo geht, geht das so wie folgt. Der Tablename muss in SingleQoutes verpackt sein, die selben die du in VB für Kommentare nimmst.

C#-Quellcode

//SqliteConnection DbConnection = new SqliteConnection($"Data Source = filename.db");
//DbConnection.Open();
private void PragmaTableInfo(string tableName)
{
using (SqliteCommand command = DbConnection.CreateCommand())
{
command.CommandText = $"PRAGMA table_info('{tableName}')";
using (SqliteDataReader reader = command.ExecuteReader())
{
for (int i = 0; i < reader.FieldCount; i++)
{
Debug.WriteLine(reader.GetName(i));
}
}
}
}

PS:
Da der Tablename ja nicht aus Userinput stammt, ist ja keine Injection möglich an der Stelle. Sqlite ist auch nur eine lokale DB, wenn da ein User eine Injection macht und alles weg ist, auch egal, sind ja seine Daten. Anders ist es wenn die DB öffentlich erreichbar ist, ich meine keinen Direktzugriff, das würde ich niemals gestatten, sondern mit einer "Zugriffsschicht" z.B. mit PHP realisiert. Dann hast du alles zu machen was geht um Injections zu vermeiden. Klar kannste auch Sqlite so öffentlich zugänglich machen, aber dafür gibt es besseres.(Postgre SQL ist aktuell mein Favorit)

Amelie · 18. Januar 2024, 09:47

@DTF

Der Code auch aus dem Ersten Beitrag funktioniert!
Ich bekomme aber diese Meldung:

Warnung CA2100 Die in 'DBConnection.LoadTableColumns(String)' an 'SQLiteCommand.CommandText.Set(String)' übergebene Abfragezeichenfolge könnte die folgenden 'DBConnection.QuoteIdentifier(tableName)'-Variablen enthalten.

Die ist nicht wegzubekommen!

Haudruferzappeltnoch · 19. Januar 2024, 05:42

Du kannst das ja mal so versuchen, statt den Tablename der Sub zu übergeben, übergibst du die Query:

VB.NET-Quellcode

Public Sub LoadTableColumns(query As String)
' Zur DB verbinden und öffnen
Con2DataBase()
Dim columnList As New List(Of String)()
Using command As New SQLiteCommand(connection)
command.CommandText = query

Dann ist der Subaufruf nicht LoadTableColumns("meineTabelle"), sondern LoadTableColumns("PRAGMA table_info('meineTabelle')").
Und da gibts dann auch keine Variable zu finden, über die er meckern kann.

--------------
Bzw. probier vorher mal New SQLiteCommand(query, connection) in deiner ursprünglichen Version, statt CommandText nachträglich zu setzen.

Amelie · 20. Januar 2024, 07:43

Es bring nichts, die Meldung geht nicht weg.
Habe sogar mit einem nochmaligen komplett neuem Project versucht. Nun ist egal, dann muss ich damit leben.
Danke

DTF · 20. Januar 2024, 08:08

Amelie schrieb:

dann muss ich damit leben.

Warnungen sind nur Warnungen, keine Fehler. Soll jetzt nicht heissen, das man Warnungen grundsätzlich ignorieren kann. Aber so einige Warnungen kann man ignorieren, solange man sich sicher ist was man tut. Hin und wieder muss man das auch absichern. Hier bist du die Person die den Tabellennamen angibt, nicht der User(Never trust User-Input). Somit kannst du dir auch die Funktion zum replacen sparen und schon wäre die Warnung weg.

Sqlite wird im normal angewendet um lokal Daten zu speichern, man braucht keinen Datenbankserver. Somit ist die Datenbank nicht öffentlich zugänglich(eine Direktverbindung sollte man dann eh meiden), immer eine Zugriffsschicht dazwischen haben. Somit sind es meist Daten vom User selbst. Wenn dieser nun meint eine Injection zu probieren, sind seine eigenen Daten weg, wenn kein Backup gemacht wurde. Deshalb sehe ich hier auch keinen Grund jedes Luftloch zuspachteln zu müssen damit es Wasserdicht ist.

ErfinderDesRades · 20. Januar 2024, 11:28

DTF schrieb:

Deshalb sehe ich hier auch keinen Grund jedes Luftloch zuspachteln zu müssen damit es Wasserdicht ist.

Aber IMO wäre schon recht wichtig, ob es Amelie überhaupt gelingt, eine parametrisierte Abfrage an SqLite abzufahren.
Zum einen um zu verifizieren, das SqLite das grundsätzlich kann (wenn auch nicht in diesem PRAGMA-Ausdruck).
Zum andern, wenn Amelie mal an eine richtige Db geht, dass sie dann nicht gezwungen ist, unverantwortlichen Bockmist zu coden.

DTF · 20. Januar 2024, 12:10

Wie es soweit aussieht kann sie das ja bereits.(Denke ich zumindest) Wobei ein wenig üben ist nicht falsch. @Amelie Wenn du das mit Sqlite soweit hinbekommen hast, kann ich dir mal empfehlen PostgreSQL zu installieren. Das ist dann eine "richtige" Datenbank. MySQL wäre auch ein Versuch wert, aber da hab ich die Erfahrung gemacht, das sich einige schwer damit tun den Server richtig aufzusetzen und dann auch zu nutzen. PostgreSQL hat auch den Vorteil, das direkt eine GUI dabei ist, mit der du die DBs bearbeiten kannst, was bei MySQL nicht der Fall ist.

Haudruferzappeltnoch · 20. Januar 2024, 17:02

ErfinderDesRades schrieb:

dass sie dann nicht gezwungen ist, unverantwortlichen Bockmist zu coden

Naja Injection ist trotz Warnung ja dennoch verhinderbar/nicht gegeben, dementsprechend ist das keineswegs "unverantwortlich".

ErfinderDesRades · 20. Januar 2024, 18:36

Tja - vlt habich was übersehen. Aber ich erinner mich an keinem Code von Amelie, der eine parametrisierte Db-Abfrage durchführt, von der sie sagt, dasses funzt.
Und falls sie das nicht kann, wird das mit grosser Wahrscheinlichkeit (kann natürlich auch sein, dass nicht...) zu unverantwortlichen Sicherheitslücken führen, falls sie mal was für andere Nutzer als sich selbst codet.

DTF · 20. Januar 2024, 19:02

Ich glaub, da hab ich was durcheinander gebracht. Dachte das hätte Amelie bereits, aber Amelie kann einmal hier gucken und damit probieren, falls sie denn spicken möchte:
SQLight Sicherheit und Codeanalysen

Weil Amelie auf meinen Post mit dem Code schrieb

Danke, aber ich habe das Problem ja nicht bei einer "Select" Abfrage sondern hier:

ging ich davon aus, das dies schon klappt. Zum üben könnte @Amelie auch noch beim hinzufügen der Parameter den Datentyp reinmachen. Gibt ja noch andere überladungen, das könnte sie eines Tages evtl. auch brauchen.

Auch ein wenig besser machen, das hier meine ich:
reader("name").ToString()

Stattdessen reader.GetOrdinal um den ColumnIndex zu bekommen, und dann mit reader.GetValue(INDEX) den Wert holen.

Amelie · 21. Januar 2024, 09:33

Moin moin

Hier mein letzter Versuch das in den Griff zu bekommen. Die Warnung bleibt bestehen.

Spoiler anzeigen

Das Form:

VB.NET-Quellcode

Public Class FrmMain
Public Sub New()
InitializeComponent()
End Sub
Private Sub FrmMain_Load(sender As Object, e As EventArgs) Handles MyBase.Load
Dim tableNames As List(Of String) = DBConnection.LoadTables()
ListBoxTables.DataSource = tableNames
End Sub
Private Sub ListBoxTables_SelectedIndexChanged(sender As Object, e As EventArgs) Handles ListBoxTables.SelectedIndexChanged
LoadColumnsForSelectedTable()
End Sub
Private Sub LoadColumnsForSelectedTable()
If ListBoxTables.SelectedItem IsNot Nothing Then
DBConnection.SelectedTable = ListBoxTables.SelectedItem.ToString()
Dim selectedTableName As String = ListBoxTables.SelectedItem.ToString()
Dim query As String = $"PRAGMA table_info({DBConnection.QuoteIdentifier(selectedTableName)})"
Dim columnsList As List(Of String) = DBConnection.LoadTableColumns(query, "@selectedInput", selectedTableName)
ListBoxCategory.DataSource = columnsList
End If
End Sub
End Class

Das Modul:

VB.NET-Quellcode

Module DBConnection
Private connection As SQLiteConnection
Private ReadOnly DataBaseName As String = "northwindEF.db"
Private _selectedTable As String
Sub New()
Con2DataBase()
End Sub
Private Sub Con2DataBase()
Dim databasePath As String = Path.Combine(Application.StartupPath, DataBaseName)
Dim connectionString As String = $"Data Source={databasePath};Version=3;"
connection = New SQLiteConnection(connectionString)
connection.Open()
End Sub
Sub CloseConnection()
If connection IsNot Nothing AndAlso connection.State = ConnectionState.Open Then
connection.Close()
End If
End Sub
Public Property SelectedTable As String
Get
Return _selectedTable
End Get
Set(value As String)
_selectedTable = value
End Set
End Property
Public Function QuoteIdentifier(identifier As String) As String
Return $"`{identifier.Replace("`", "``")}`"
End Function
Public Function LoadTables() As List(Of String)
Dim tableNames As New List(Of String)()
Using command As New SQLiteCommand("SELECT name FROM sqlite_master WHERE type='table' AND name <> 'sqlite_sequence'", connection)
Using reader As SQLiteDataReader = command.ExecuteReader()
While reader.Read()
Dim tableName As String = reader("name").ToString()
tableNames.Add(tableName)
End While
End Using
End Using
CloseConnection()
Return tableNames
End Function
Public Function LoadTableColumns(query As String, ByVal parameterName As String, ByVal parameterValue As Object) As List(Of String)
Con2DataBase()
Dim columnList As New List(Of String)()
Using command As New SQLiteCommand(query, connection)
command.Parameters.AddWithValue(parameterName, parameterValue)
Using reader As SQLiteDataReader = command.ExecuteReader()
While reader.Read()
Dim columnName As String = reader("name").ToString()
columnList.Add(columnName)
End While
End Using
End Using
CloseConnection()
Return columnList
End Function
End Module

ErfinderDesRades · 21. Januar 2024, 14:20

sag bescheid, wenn du lernen möchtest, parametrisierte DbCommands zu verwenden.
Dass die PRAGMA-Anweisung dafür nicht geeignet ist, hat sich ja nu herauskristallisiert.
Das ist aber auch eine sehr ungewöhnliche Abfrage - normalerweise kennt man die Spalten der DbTabelle ja bereits.

Also mit anderen Abfragen könntest du lernen, parametrisierte DbCommands zu verwenden.

Amelie · 22. Januar 2024, 06:52

Also um ein bissel mehr Licht ins dunkel zu bringen, auch was das "lernen" betrifft.

Ich habe mal ein kleines Tool für meinen Opa geschrieben. Läuft auf seinem Laptop ( meist ohne Internet ) Dieses Tool basiert auf vielen XML-Dateien und ist, sagen wir mal so, sehr zusammengestückelt, keine Klassen, kein OOP usw. Davon wollte ich nun ein "Rebuild" mit Datenbank usw. schreiben.

Begonnen hatte ich dann mit Access 2003 weil das bei Opa auf dem Laptop läuft und die Anbindung an VB.net einfach ist. Dann kam der große "Knackpunkt" das ganze ginge dann nur mit 32Bit.
Also als alternative ohne ins System einzugreifen, war dann "SQLight". Die GUI dazu finde ich etwas seltsam und tue mich echt schwer da die DB / Tabellen und Beziehungen zu erstellen. Dann ist die Anbindung an VB auch nicht so einfach. Also das mit dem Designer DT / DS usw.. Ich wollte mir aber auch nicht gleich wieder haufenweise neue Sachen von "NuGet" installieren, die im Moment noch nicht in meinen Skill passen.

Das mit der Codeanalyse wollte ich halt machen, um zu sehen, ob das mit meinem "CleanCode" usw nun besser geworden ist.
Danke

ErfinderDesRades · 22. Januar 2024, 08:36

Amelie schrieb:

ob das mit meinem "CleanCode" usw nun besser geworden ist.

ConToDatabase() kann weg - der Code sollte in Sub New ausgeführt werden, ausser: connection.Open(). Das sollte da aufgerufen werden, wo die connection geöffnet werden soll.
CloseConnection() kann weg - stattdessen reicht der direkte Aufruf: connection.Close().
Wozu Property SelectedTable erforderlich ist ist nicht ersichtlich.
LoadTableColumns() führt eine beliebige Query aus - macht somit etwas ganz anderes als die Function heisst.
Würde LoadTableColumns() mit einem geeigneten Query-String aufgerufen, so würde sie ein parametrisiertes Command mit einem Parameter ausführen.
Wird aber mit einem ungeeigneten Query-String (ohne Parameter-Platzhalter) aufgerufen, also macht nochmal was anderes, als man anderes denkt, was beabsichtigt sein könnte.

SQLight Sicherheit und Codeanalysen

Haudruferzappeltnoch schrieb:

VB.NET-Quellcode

VB.NET-Quellcode

VB.NET-Quellcode

C#-Quellcode

VB.NET-Quellcode

Amelie schrieb:

DTF schrieb:

ErfinderDesRades schrieb:

VB.NET-Quellcode

VB.NET-Quellcode

Amelie schrieb:

Benutzer online 3

Tags

Ähnliche Themen

6 Benutzer haben hier geschrieben