Reading SQL column name from querystring and building secure query from it

Question

I am building in C#/.NET2.0 a page that updates different columns dynamically in SQL call for example: myajaxpage.aspx?id=1111&fieldname=title

What is the correct way to build SQL query for reading column name from querystring? Is this good approach in a first place?

I tried:

cmd.CommandText = "UPDATE MyTable SET +"Request.QueryString["fieldname"]"+ = @fieldvalue WHERE id = @id";

Which works but is not secure, can you please advice how to make this query secure?

Answer 1

not at all , you gotta have a list of possible columns , unless the users can enter whatever they want

var possibleColumns = New List<String>; // add pssobile columns here
if (possibleColumns.Contains(Request.QueryString["fieldname"])
   cmd.CommandText = "UPDATE MyTable SET +"Request.QueryString["fieldname"]"+ = @fieldvalue WHERE id = @id";
else
   Response.Write(" invalid query ");

Answer 2

Use a SQL stored procedure instead of an ad-hoc SQL statement.

Answer 3

Definitely do not append a querystring param into the SQL like that. If you're going to do it, instead support a list of "friendly" column names in the querystring which you then map onto real field names in your table - so you're also then not exposing anything about your actual schema to the outside world.

Answer 4

First, consider if there is another way to do this: exposing actual column names is probably a bad idea. Now a malicious user has just that much less work to do.

That said, I would consider validating your input against an expected list of values. If the value for fieldname is something you weren't expecting, you should abort before it reaches the database layer.

Finally, you should consider using square brackets to quote the field name. If a closing square bracket is found in the input, escape it by replacing it with a double closing square bracket:

[this [should]] be a valid name too]

Answer 5

You should abstract the field name away from the querystring altogehter. The querystring could contain some identifier that represents the field, but there is no reason to expose details of the database layout outside of the server.

Anything that comes from a browser should never be used directly in a query without verifying it. You have to verify that the field name is one of the field that you allow changing, so it's about the same amount of work to translate an independent identifier to the actual field name.

Answer 6

Hi there.

Use the SqlParamterCollection.Add() method, as given in this example. You can still call the T-SQL as a text command, but you can add paramters to the SQL.

sqlCmd.CommandType = CommandType.Text;
sqlCmd.CommandText = string.Format("UPDATE MyTable SET {0} = @fieldvalue WHERE id = @id", Request.QueryString["fieldname"]);

sqlCmd.Parameters.Add("@fieldvalue", SqlDbType.VarChar).Value = "Some Value";
sqlCmd.Parameters.Add("@id", SqlDbType.Int).Value = "34";

You can also specify the size of the parameter, which is handy for string params.

sqlCmd.Parameters.Add("@fieldvalue", SqlDbType.VarChar, 10).Value = "Some Value";

So anyone trying to put a cheeky SQL statement into the @fieldValue param woudl be hindered by the max 10 chars allowed for the value. This can help limit the possibility of SQL injection attacks. Here's a good link regards SQL injection and .NET code.

In response to comment I got, you might want to verify the request querystring parameter, in case it contains potentially harmful SQL:

string fieldName = HttpUtility.UrlDecode(Request.QueryString["fieldname"]);

if (!fieldName.Contains(" "))
{
   // Do the rest of the sql code here...
}

So the idea is, if the parameter has a space, then it can't be a valid field name, so could potentially contain dangerous SQL.

Jas.