Is this Sql-injection-proof Asp.net code?

Question

Problem: I have a form with text values, and a function that must return a string query based on the values of the text values too.

Solution: I created a SQLCommand query with parameters, then I put the SQLCommand.CommandText to a string and I returned it (to the business logic that is going to handle the query)

Main Question: Is it sql-injection proof?

Code Example:

sQuery = "select * from xy where x like '%@txtNameParameter%'";

SqlCommand cmd = new SqlCommand(sQuery);

cmd.Parameters.Add("@txtNameParameter", SqlDbType.VarChar);
cmd.Parameters["@txtNameParameter"].Value = txtName.Text;

string query = cmd.CommandText;
return query;

Sub question if main question is ok: Should I put into parameters also values of a radiobutton and dropdownmenu or are they injection-proof?

Once

The function (?) above returns only the command text and the command object itself is abandoned? WHy not return the command object? – Propagate

Because I need to return only the where clause of the query. So what i do before returning it is to substring it deleting the select part. This because then the string query will be sent to a business logic that already has the select part, and takes as input the string with the where clauses that I have to injection-proof here. – Once

Better would be to send the values of the controls into the business layer, which in turn can decide how the data access should be driven. It can place the values into a SQL Command that it can actually use instead of passing T-SQL Snippets around which will likely (and quickly) get out of control – Upheld

I know, and I would do that, but I don't have the control of the data access and I have to handle this situation. Is the query that I return safe? – Once

If you don't have control of the data access layer, how do you expect to inject a WHERE clause into it? (because that it what you are eventually doing) – Extortionate

I agree with @Colin Mackay - if the data layer is allowing you to pass arbitrary WHERE clauses in that is very unusual. If that is the case though then you might have to go for old fashioned approaches of things like escaping apostrphes and anything else necessary. You might also want to talk to whoever does have control of the DAL and ask them how you are meant to handle this properly... – Cachepot

Answer 1

What you are doing here is injection proof because you are not injecting anything. In fact, your parameter isn't even used (because the only reference to it is inside a string literal so the SQL Parser won't even see where you are attempting to use the parameter because it will treat it as a string literal.)

You may want to change that line of code to:

sQuery = "select * from xy where x like '%'+@txtNameParameter+'%'";

Which would make the SQL look like this:

select * from xy where x like '%'+@txtNameParameter+'%'

Which is just string concatenation in a place where a string is expected in the SQL command anyway.

However, your description of what you are doing with this afterwards possibly blows all that out of the water. I cannot understand why you would want to send just the where clause of the query to the business layer.

Also, the substringed WHERE clause will not contain the data you are putting in the parameter. So you are getting no more benefit that just returning

return "where x like '%@txtNameParameter%'";

The parameter value is lost.

Extortionate

An excellent answer. The last bit being the particularly relevant point that the parameters aren't interpreted in the code at all, only when they are sent to the database so putting it all into a command is doing nothing. – Cachepot