ZewoGraveyard/SQL

Vulnerable to SQL injection

C0deH4cker opened this issue · 4 comments

Client code using this SQL library is extremely likely to be vulnerable to SQL injection attacks. As can be seen here in the Connection class, the .execute() method has only a single argument of type String. That means that client code is likely to call this method like db.execute("SELECT * FROM table WHERE username='\(username_param)'"), which is trivially exploitable via SQL injection. Even the provided sample code follows this pattern and is therefore vulnerable to SQL injection.

For example, let's say a user of the sample REST APIv2 sent the following request:

DELETE /todos/1'or''='

This would expand on line 50 of TodoRepository.swift to the following SQL statement to execute:

DELETE from todos WHERE id = '1'or''=''

The WHERE clause will always be true since an empty string is equal to an empty string, and therefore that will delete the entire database.

It is true that this is the fault of the client code (in this case, the provided example code). However, there is no secure option provided by the Connection class. Most modern SQL libraries have a way of creating parameterized statements to execute. This works similarly to the printf() function from C, in that you provide a "pattern" string to the API function along with placeholders where data will be stored, and then you provide the API function with the data values. Internally it will ensure that the provided SQL pattern cannot be modified by user input to execute different SQL code than desired. For an example of what I mean, see https://www.owasp.org/index.php/Query_Parameterization_Cheat_Sheet for some examples of SQL APIs that allow for parametrized queries.

Closing this issue as PostgreSQL uses PQExecParams, and MySQL will escape all text params. It's still possible to inline parameters in swift using \() but this is out of our hands.

👍

Agreed, thanks for addressing this so quickly!