Is SQL injection possible with this query?

Question

UPDATE `company` SET `itnumber` = '595959' WHERE (id = 932)

So the value of itnumber is coming from user input for that company. I want to make sure I am able to prevent any kind of sql injection. So user inputs 595959 and I build that value as '595959' in the dynamic query. Is it still possible to have sql injection attack in this query? I am aware of using prepare statement to use to prevent sql injection, but prepare statement might take a lot of development efforts for my application, so I am looking for less time consuming and easier approach to fix most of my sql statements where injection is possible.

StringBuffer sb = new StringBuffer();
sb.append(" UPDATE ");
sb.append(DB.quote(table));
sb.append(" SET ");
/* logic if column value has changed */
/* if yes */
sb.append(DB.quote(column.name));
sb.append(" = ");
sb.append(column.getSQLvalue());
sb.append(" WHERE (id = ");
sb.append(columns[0].getSQLvalue());
sb.append(")");
execute(sb.toString());

Analyzing whether SQL injection is possible requires looking at the *source* that generates the SQL, not the resulting SQL. — Gabe, Feb 28 '11 at 20:31
@Alexander Pogrebnyak - at a guess, the SQL string is probably constructed in a java application. — Oded, Feb 28 '11 at 20:36
We need the dynamic SQL that generated your query, among other things. — Lamak, Feb 28 '11 at 20:37
@Joe: ah, little Bobby Tables. Probably my favourite xkcd of all time. — Sean Reilly, Feb 28 '11 at 20:38
The time learning to use Prepare Statements is well worth it, and not actually that long. It's like saying "I don't have time to learn how to make cement, can I build a house with super-glue instead?" :) — MatBailie, Feb 28 '11 at 21:17

Oded · Accepted Answer · 2011-02-28T20:56:14.477

If you are simply concatenating the input into a SQL string without doing any cleanups (and simply surrounding it with single quotes ' doesn not make it clean), then yes, it is vulnerable to SQL injection.

Please post the code that constructs this SQL for a definitive answer.

Update:

Since you are using getSQLvalue() from the Oracle SQL library, this would ensure that the value passed in is escaped correctly. This is indeed safe from SQL injection, however it requires you to remember to use it in every place. Using parameters would ensure the same, however, without the risk of forgetting to escape your SQL values.

score 3 · Answer 2 · answered Feb 28 '11 at 20:35

3

Yes, it is. For example:

UPDATE `company`
SET `itnumber` = '595959'; DROP TABLE company; --' WHERE (id = 932)

Would probably work.

answered Feb 28 '11 at 20:35

Sean Reilly

21,526
4
48
62

I tried this, but it stored the value of 595959';DROP TABLE company;-- for `itnumber` field. – yogsma Feb 28 '11 at 20:49
It depends on the type of database you are updating and the way you build the dynamic SQL string in code. Do you have an example that you can show us? – Sean Reilly Feb 28 '11 at 20:58

score 3 · Answer 3 · answered Feb 28 '11 at 20:45

I am aware of using prepare statement to use to prevent sql injection, but prepare statement might take a lot of development efforts for my application, so I am looking for less time consuming and easier approach to fix most of my sql statements where injection is possible.

You would be surprised how much less time consuming it is to implement solution the right way from the start. Plus how come this is more complex than concatenating query string?

PreparedStatement pstmt = con.prepareStatement(
       "UPDATE `company` SET `itnumber` = ? WHERE (id = ?)"
   );
pstmt.setString(1, "595959");
pstmt.setInt(2, 932);

Plus, it has an advantage that most modern drivers will cache the execution plan of the prepared statement, thus this will speed up other queries.

Christopher Samulski · Answer 4 · 2011-02-28T21:23:34.047

Really the safest thing to do is create a stored proc for this, then your datatypes at least protect you a little.

CREATE PROC usp_Update_itnumber_by_Company_Id

@itnumber int
, @Company_Id int

as

BEGIN TRAN    
UPDATE [Company]
SET itnumber = @itnumber
WHERE id = @Company_Id;

COMMIT TRAN

At this way if if ; DROP TABLE company; --' WHERE (id = 932) is passed into @Company_Id it will fail as the datatype is a miss match.

If you're trying to determine which columns to update and only if they've changed you could look into doing a MERGE instead of an UPDATE statement. This way you can pull the current state of a record and only update if they arne't the same.

Is SQL injection possible with this query?

4 Answers4