Strings

Jon Skeet [C# MVP]

On Sep 18, 12:00 pm, AA2e72E <>
wrote:
> I take your point. BUT I do not understand how the approach I proposed is
> MORE vulnerable to SQL injection attacks than an approach that uses SQL
> Command

Your strategy builds up a literal SQL string containing user input.
Unless you are 100% accurate in escaping any values entered by the
user, that SQL string could have significant unintended consequences.
I know I don't trust myself to be able to escape SQL values accurately
in every possible case - do you?

I *do*, however, trust the author of the API to have got it right -
either by doing the proper escaping, or more likely by shipping the
values separately, so they're never, ever part of a direct SQL command
and will never risk being executed as SQL.

> 1 .... in an EXE or DLL i.e the user interface does not allow execution of
> SQL statements directly i.e does not have a 'Query Analyser' type facility.
> 2. Someone with malicious intent can easily use Reflector.exe to reverse
> engineer all the code and other particulars to ...destroy...

Neither of those are relevant to SQL injection attacks.

> Given what is said here, I'll review my strategy; however, I have several
> complex and long SQL statements (20+lines long) stored in resource files, the
> database connection is passed in to my DLL to a private object; the
> application needs to run the SQLs with different parameters.

None of that stops you from using parameterized queries. It sounds
like you may have misconceptions about what parameterized queries are
like - they're very similar (in use) to your strategy: you still have
placeholders, and specify the values separately. It's just that
instead of formatting the values into the SQL string, you ask the
database client to handle them.

Jon

Ben Ryves

Use the static string.Format() method, ie:

string let = string.Format("Dear Mr {0} \r\nThank you for your letter of
{1}","CSHARP","01/01/2007");

"AA2e72E" <> wrote in message
news:...
> I have seen code like this:
>
> Console.WriteLine(
> "divisible by seven event raised!!! the guilty party is {0}",
> e.TheNumber);
>
> where {0} gets substituted by e.TheNumber
>
> Is there a general way of doing this. e.g.
>
> "Dear Mr {0} \r\nThank you for your letter of {1}","CSHARP","01/01/2007"
>
> and have {0} replace by CSHARP and {1} replaced by 01/01/2007 and then
> have
> the new string returned , e.g
>
> string let = "Dear Mr {0} \r\nThank you for your letter of
> {1}","CSHARP","01/01/2007"
>
> Thsnks for your help.
>

Chris Shepherd

> None of that stops you from using parameterized queries. It sounds
> like you may have misconceptions about what parameterized queries are
> like - they're very similar (in use) to your strategy: you still have
> placeholders, and specify the values separately. It's just that
> instead of formatting the values into the SQL string, you ask the
> database client to handle them.

To elaborate on Jon's post with an example:

string group = "CarWidgets";
string type = "A";
string sqlQuery = "SELECT widgetId, description, createdDate FROM
Widgets WHERE widgetGroup = @group AND widgetType = @type";

SqlConnection conn = new SqlConnection(/*connection info*/);
SqlCommand cmd = new SqlCommand(sqlQuery,conn);
cmd.Parameters.AddWithValue("group", group);
cmd.Parameters.AddWithValue("type", type);
// Get Data via DataAdapter, SqlReader, etc...

The above lets the SqlClient classes deal with any kind of escaping or
conversions that need to go on, and in my experiences is far less prone
to bugs.


The old/bad (IMO anyway) way is to do something like:

string group = "CarWidgets";
string type = "A";
string sqlQuery = "SELECT widgetId, description, createdDate FROM " +
"Widgets WHERE widgetGroup = '" + group + "' AND widgetType = '" +
type + "'";

SqlConnection conn = new SqlConnection(/* connection info */);
SqlCommand cmd = new SqlCommand(sqlQuery, conn);
// Get Data via DataAdapter, SqlReader, etc...

This puts the onus on you, rather than the API to make the right call in
how you handle input sanitizing, for instance. There also may be some
conversions that should/have to be done on data types before storage
that can be taken care of by the client.

In my experience it's far better to rely on the underlying code that
deals with querying the database than it is to have to write that stuff
on your own.

Chris.

AA2e72E

Thanks for the example; I take the point.

I agree that what you describe as the "old/bad" way is bad even though it is
much better with C# than used to be with VB 6.0. That is what I was trying to
avoid.

I can see the advantages of SQLCommand and using parameters. You, like Jon,
mention "escaping": what is that?

Jon Skeet [C# MVP]

On Sep 18, 2:10 pm, AA2e72E <> wrote:
> Thanks for the example; I take the point.
>
> I agree that what you describe as the "old/bad" way is bad even though it is
> much better with C# than used to be with VB 6.0. That is what I was trying to
> avoid.
>
> I can see the advantages of SQLCommand and using parameters. You, like Jon,
> mention "escaping": what is that?

It's converting an input string into a safe format. For instance,
suppose you have this SQL command:

SELECT * from Customers WHERE ID='{0}'

Consider what would happen if {0} were replaced with the following
user string:
ID'; DROP TABLE Customers; SELECT * FROM USERS WHERE ID='

Your single SQL statement has become three, one of which will drop a
table! Escaping would double the quotes so that the server would treat
the whole lot as a single value. However, it's very hard to do
properly for all situations. You don't need to worry about this if
you're using SqlCommand and specifying parameters instead of
formatting the values into the SQL string itself.

Jon

Chris Shepherd

AA2e72E wrote:
> Thanks for the example; I take the point.
>
> I agree that what you describe as the "old/bad" way is bad even though it is
> much better with C# than used to be with VB 6.0. That is what I was trying to
> avoid.
>
> I can see the advantages of SQLCommand and using parameters. You, like Jon,
> mention "escaping": what is that?

Let's say you use the second query I posted. By default, that code is
vulnerable if the group variable is passed in, instead of set just above
the code block. Another example:

string group = "' haha it broke";

If the value of group contains a quotation mark, the query then becomes:

SELECT widgetId, description, createdDate FROM Widgets WHERE widgetGroup
= '' haha it broke' AND widgetType = 'A'

The problem here is the two single quotes will break.

Worse yet, a user could make group = "' --;". That may not seem bad
right, it's just an SQL error. However, consider this is your login
query and you handle authentication on your own:

public void auth(string user, string pass)
{
string sql = "SELECT username FROM users WHERE username = '" +
user + "' AND pass = '" + pass + "'";

// Check to see if we got the result
}

Now, let's say there's no input sanitizing going on, and a malicious
user puts in: "admin' --" as their username. The resulting query would
look like:

SELECT username FROM users WHERE username = 'admin' --' AND pass = ''

This results in it finding the users entry that matches just the
username, without the password being taken into account at all.
Obviously very very bad.

Anyhow, to the point, escaping is something done on a variety of SQL
server clients that basically takes unsafe characters and escapes them
similarly to newlines (\n).

If you did the above with parameters, the database client will escape
the values so that the last one would be:

SELECT username FROM users WHERE username = 'admin\' --' AND pass = ''

Some implementations will use '' instead of \', but ultimately to the
database server it will see that the user is trying to match "admin' --"
to a username, and probably fail.

Chris.

Göran Andersson

AA2e72E wrote:
> Thanks for the example; I take the point.
>
> I agree that what you describe as the "old/bad" way is bad even though it is
> much better with C# than used to be with VB 6.0. That is what I was trying to
> avoid.
>
> I can see the advantages of SQLCommand and using parameters. You, like Jon,
> mention "escaping": what is that?

Escaping is commonly used for replacing certain characters inside a
string that has a special meaning where the string is used, for example
html encoding or url encoding.

In SQL strings, the apostrophe (') has to be escaped, and in some
dialects also the backslash (\). Depending on dialect, they are escaped
differently.

--
Göran Andersson
_____

Marc Gravell

Just additional; escaping (to avoid injection) is clearly a big win,
but so is disambiguation...

For instance, if I pass the following literal as a date, when is it?
"01/02/03"
OK - now how about if your user, your web-server, and your database-
server are all in different cultures?

Now if I pass a DateTime instance instead (via a parameter), then the
database server is given the suitable epoch-value, rather than a
string to parse.

Likewise, numbers; are the following 1 point 5, or 1 thousand 5
hundred? "1.500", "1,500"
It depends where you are ;-p
Passing primatives (as parameters) rather than string literals avoids
this type of data error.

Marc

Cor Ligthert[MVP]

Hi Jon,

I was pleased to see your first answer in this thread, although you say it
often, this shows that you go to: The, maintenance is more important then 1
millisecond, way.

However after that message I lost the relation to the question. What has a
SQL parameter to do with replacing words in by instance the date on a
letter?

Really curious

Cor

"Jon Skeet [C# MVP]" <> schreef in bericht
news:1190111398.403052.175790@g4g2000hsf.googlegro ups.com...
> On Sep 18, 11:06 am, AA2e72E <>
> wrote:
>> String concatenation on a piecemeal basis, I think, creates clutter and
>> make
>> the code difficult to follow; I have in mind to use this substitution
>> technique as follows:
>>
>> 1. hold an SQL statement with {n}
>> 2. substitute the n values in
>>
>> and then execute it.
>
> No, please don't do that. That will still build a literal piece of
> SQL, which is likely to be vulnerable to SQL injection attacks.
> Instead, use a parameterized SQL command, and get client API to
> perform any substitutions it needs to (or more likely, send the values
> separately).
>
> The exact form of this will depend on which database you're talking
> to, but it's *definitely* the way to go.
>
> Jon
>

Jon Skeet [C# MVP]

Cor Ligthert[MVP] <> wrote:
> I was pleased to see your first answer in this thread, although you say it
> often, this shows that you go to: The, maintenance is more important then 1
> millisecond, way.

Absolutely. Readability is king, until you've proved performance
issues.

> However after that message I lost the relation to the question. What has a
> SQL parameter to do with replacing words in by instance the date on a
> letter?

The OP was using it as an example, but his real use was to try to
replace parameters within a SQL statement. So he was going to have
something like:

SELECT * FROM Orders WHERE Customer = '{0}'

--
Jon Skeet - <>
Blog:
If replying to the group, please do not mail me too