Can This Code Be Exploited for SQL Injection with Significant Impact?
0
votes
2
answers
92
views
I’m currently debating with a colleague whether the following (pseudo) code is vulnerable to SQL injection (SQL Server):
database.BeginTransaction();
String userId = dto.UserId;
String firstQuery = "select * from users where userid='" + userId + "'";
ResultSet rs = database.executeQuery(firstQuery);
// Use the first entry in the result set as a user with 10 columns
rs.next();
User user = new User();
user.Username = rs.getString(2);
...
// Character-wise comparison of users password with dto.Password, if incorrect, unauthorized is returned here
// Now the second query, which doesn’t need a return but must run successfully
String secondQuery = "select * from settings where userid='" + userId + "'";
ResultSet rs = database.executeQuery(secondQuery);
// Use the first entry in the result set as settings with 40 columns
rs.next();
Settings settings = new Settings();
settings.DarkMode = rs.getBoolean(2);
...
database.CommitTransaction();
// catch block here
In reality, this code looks quite bad. I wanted to prove to my colleague that it’s vulnerable to SQL injection, and I managed to exploit it using UNION ALL
in the first query to retrieve a user with a custom password like this ' UNION SELECT TOP 1 UserID, Username, 'MyCustomPassword' AS Passwort, ... FROM Users --.. However, the second query fails due to a column count mismatch (10 vs. 40 columns), throwing an error when UNION ALL
is executed. The result of the second query is not important but the query should not throw. Is there any sql string for userid that would satisfy both queries syntax? Is this code really "safe"
I also tried to insert a user into the table by concatenating a SELECT
with an INSERT
statement, but since CommitTransaction
is never called, nothing actually happens. So while this code is indeed vulnerable to SQL injection, the transaction wrapping prevents any serious impact. This feels somewhat absurd—am I missing something?"
EDIT: The userId is coming from the client and the transaction is rollbacked in the catch block therefore my initial approach with insert did not seem to work since the outer transaction is rollbacked when the second query is run (throws error) and therefore the transaction within the injected sql, or am I missing something?
Asked by D.Dave
(1 rep)
Oct 16, 2024, 03:24 PM
Last activity: Oct 16, 2024, 04:19 PM
Last activity: Oct 16, 2024, 04:19 PM