93

I use an API that expects a SQL string. I take a user input, escape it and pass it along to the API. The user input is quite simple. It asks for column values. Like so:

string name = userInput.Value;

Then I construct a SQL query:

string sql = string.Format("SELECT * FROM SOME_TABLE WHERE Name = '{0}'",
                           name.replace("'", "''"));

Is this safe enough? If it isn't, is there a simple library function that make column values safe:

string sql = string.Format("SELECT * FROM SOME_TABLE WHERE Name = '{0}'",
                           SqlSafeColumnValue(name));

The API uses SQLServer as the database.

Improve this question
8
  • Parameterise it! A Google search of SO for "SQL injection" Edit: In response to Seva Alekseyev, SO answer with character 8 injection Commented Mar 8, 2010 at 18:27
  • 3
    You need to use parameters. Commented Mar 8, 2010 at 18:30
  • 2
    @SLaks, Obvious the API doesn't allow it. Maybe he needs a new API. Commented Mar 8, 2010 at 18:48
  • 3
    @sri you'll get better answers if you explain why you can't/won't use parameters. Commented Mar 9, 2010 at 5:46
  • 2
    @MladenB. Legacy design exists, and replacing entire systems with "best practice" code isn't always viable. Whilst I agree that this should be param based and decoupled, there are tons of reasons that this may not be the case. Try getting a business to invest in refactoring an API which has been working for 10 years, has a cost attributed to "fixing" it and carries high risk if changed! Commented Dec 22, 2020 at 9:13

6 Answers 6

Reset to default
171

Since using SqlParameter is not an option, just replace ' with '' (that's two single quotes, not one double quote) in the string literals. That's it.

To would-be downvoters: re-read the first line of the question. "Use parameters" was my gut reaction also.

EDIT: yes, I know about SQL injection attacks. If you think this quoting is vulnerable to those, please provide a working counterexample. I think it's not.

Improve this answer
Sign up to request clarification or add additional context in comments.

14 Comments

There's a little ' after Robert. That's how. When properly quoted, Bobby's unorthodox name would stored in the database in its entirety.
Sure... character 8 injection: stackoverflow.com/questions/1800013/…
Seva Alekseyev thank you so, so, so much for reading my question and trying to answer it. Thank you for not preaching to me about "best practices". And thank you for not trying to "educate me" Really, thank you!
Um, for the record, I still think that parameters are the way to go :) However, I've been in this business long enough to understand the existence of constraints on your designs. Been there, done that.
@Seva While everyone knows the code above is subject to injections, sometimes you are tied to an API that just doesn't allow you to follow best practices. Too often programmers worry about stuff that isn't within the circle of concern of a particular task. Good answer to the specific question.
|
1

SqlCommand and Entity Framework use exec sp_executesql....

So there really is an alternative to raw strings with your own escaping pattern presumably. With SqlCommand you are technically using parameterised queries but you're bypassing the ADO.Net abstraction of the underlying SQL code.

So while your code doesn't prevent SQL Injection, the ultimate answer is sp_executesql not SqlCommand.

Having said that, I'm sure there are special handling requirements for generating an SQL Injection-proof string which utilizes sp_executesql.

see: How to return values from a dynamic SQL Stored Procedure to the Entity Framework?

Improve this answer

Comments

0

I was using dynamic sql (I can hear the firing squad loading their rifles) for search functionality, but it would break whenever a user searched for somebody with a surname like "O'Reilly".

I managed to figure out a work-around (read "hack"):

Created a scalar-valued function in sql that replaced a single quote with two single quotes, effectively escaping the offending single quote, so
"...Surname LIKE '%O'Reilly%' AND..." becomes "...Surname LIKE '%O''Reilly%' AND..."

This function gets invoked from within sql whenever I suspect fields could contain a single quote character ie: firstname, lastname.

CREATE FUNCTION [dbo].[fnEscapeSingleQuote]
    (@StringToCheck NVARCHAR(MAX))
RETURNS NVARCHAR(MAX)
AS
BEGIN
    DECLARE @Result NVARCHAR(MAX)
    SELECT @Result = REPLACE(@StringToCheck, CHAR(39), CHAR(39) + CHAR(39))
    RETURN @Result
END

Not very elegant or efficient, but it works when you're in a pinch.

Improve this answer

Comments

0

One may wish to replace ' with '' instead of parameterizing when needing to address the ' problem in a large amount of ad hoc sql in a short time with minimal risk of breakage and minimal testing.

Improve this answer

Comments

-6

Simple:

const string sql = "SELECT * FROM SOME_TABLE WHERE Name = @name";

and add the @name parameter with value:

cmd.CommandText = sql;
cmd.Parameters.AddWithValue("@name", name);
Improve this answer

3 Comments

He still needs to get the actual SQL statement out of the command object to pass to the API.
He explicitly states that he is using an API that accepts a SQL string. He is NOT using ADO.Net.
@MHollis exactly; which is the first problem they need to fix. Sorry, but there is no advisable way of doing this without parameters. There is a reason folks are shouting about parameters.
-11

If you need to escape a string for a MSSQL query try this:

System.Security.SecurityElement.Escape(Value)
Improve this answer

1 Comment

Sorry Kevin but that Escape function only escapes XML not SQL (msdn.microsoft.com/en-us/library/…)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.