WPF/SQL: Replacing values with parameters?

Question

I am new to C# and I've written the following code. After reading through the documentation I realized that my code is supposedly vulnerable to SQL injections, because I am not using parameters for my values (as far as I understand, you could inject unwanted queries through the search.Text). Should I even worry about it, as I am essentially locking my values inside the "" quotation marks anyway?

I found some directions here, but I can't get it to work: How to use string variable in sql statement

public void InvokeDataGridAddress()
{
    switch (ComboBoxSelection.Text)
    {
        case "NASLOV":
            comboBoxValue = "SELECT * FROM [cbu_naslovi] WHERE [ADDRESS] LIKE '%" + search.Text + "%' COLLATE Latin1_general_CI_AI";
            break;
        case "LASTNIK":
            comboBoxValue = "SELECT [cbu_naslovi].* FROM [cbu_deli], [cbu_naslovi] WHERE [cbu_deli].LASTNIK LIKE '%" + search.Text + "%' COLLATE Latin1_general_CI_AI AND [cbu_deli].IDX = [cbu_naslovi].ID";
            break;
        case "OBJEKT":
            comboBoxValue = "SELECT * FROM [cbu_naslovi] WHERE [SO] LIKE '%" + search.Text + "%'";
            break;
        case "PARCELA":
            comboBoxValue = "SELECT * FROM [cbu_naslovi] WHERE [P1] LIKE '%" + search.Text + "%' OR [P2] LIKE '%" + search.Text + "%' OR [P3] LIKE '%" + search.Text + "%' OR [P4] LIKE '%" + search.Text + "%' OR [P5] LIKE '%" + search.Text + "%' OR [P6] LIKE '%" + search.Text + "%' OR [P7] LIKE '%" + search.Text + "%' OR [P8] LIKE '%" + search.Text + "%' OR [P9] LIKE '%" + search.Text + "%' OR [P10] LIKE '%" + search.Text + "%' OR [P11] LIKE '%" + search.Text + "%' OR [P12] LIKE '%" + search.Text + "%' OR [P13] LIKE '%" + search.Text + "%' OR [P14] LIKE '%" + search.Text + "%' OR [P15] LIKE '%" + search.Text + "%' OR [P16] LIKE '%" + search.Text + "%' OR [P17] LIKE '%" + search.Text + "%'";
            break;
    }
    comboBoxValue = comboBoxValue + " ORDER BY [ULICA] ASC, [OBMOCJE] ASC, LEN ([HS]) ASC, [HS] ASC, [HID] ASC";
    SqlCommand cmd = new SqlCommand
    {
        CommandText = comboBoxValue,
        Connection = con
    };
    Mouse.OverrideCursor = System.Windows.Input.Cursors.Wait;
    SqlDataAdapter da = new SqlDataAdapter(cmd);
    dtAddress.Clear();
    da.Fill(dtAddress);
    dg_address.ItemsSource = dtAddress.DefaultView;
    Mouse.OverrideCursor = System.Windows.Input.Cursors.Arrow;
}

EDIT: Working solution made possible by Olivier Belanger and MindSwipe. I'm also leaving a reference about how to make LIKE work with % parameters: Use of SqlParameter in SQL LIKE clause not working

public void InvokeDataGridAddress()
{
    switch (ComboBoxSelection.Text)
    {
        case "NASLOV":
            comboBoxValue = "SELECT * FROM [cbu_naslovi] WHERE [ADDRESS] LIKE @SearchText COLLATE Latin1_general_CI_AI";
            break;
        case "LASTNIK":
            comboBoxValue = "SELECT [cbu_naslovi].* FROM [cbu_deli], [cbu_naslovi] WHERE [cbu_deli].LASTNIK LIKE @SearchText COLLATE Latin1_general_CI_AI AND [cbu_deli].IDX = [cbu_naslovi].ID";
            break;
        case "OBJEKT":
            comboBoxValue = "SELECT * FROM [cbu_naslovi] WHERE [SO] LIKE @SearchText";
            break;
        case "PARCELA":
            comboBoxValue = "SELECT * FROM [cbu_naslovi] WHERE [P1] LIKE @SearchText OR [P2] LIKE @SearchText OR [P3] LIKE @SearchText OR [P4] LIKE @SearchText OR [P5] LIKE @SearchText OR [P6] LIKE @SearchText OR [P7] LIKE @SearchText OR [P8] LIKE @SearchText OR [P9] LIKE @SearchText OR [P10] LIKE @SearchText OR [P11] LIKE @SearchText OR [P12] LIKE @SearchText OR [P13] LIKE @SearchText OR [P14] LIKE @SearchText OR [P15] LIKE @SearchText OR [P16] LIKE @SearchText OR [P17] LIKE @SearchText";
            break;
    }
    comboBoxValue = comboBoxValue + " ORDER BY [ULICA] ASC, [OBMOCJE] ASC, LEN ([HS]) ASC, [HS] ASC, [HID] ASC";
    SqlCommand cmd = new SqlCommand
    {
        CommandText = comboBoxValue,
        Connection = con
    };
    cmd.Parameters.AddWithValue("@SearchText", '%' + search.Text + '%');
    Mouse.OverrideCursor = System.Windows.Input.Cursors.Wait;
    SqlDataAdapter da = new SqlDataAdapter(cmd);
    dtAddress.Clear();
    da.Fill(dtAddress);
    dg_address.ItemsSource = dtAddress.DefaultView;
    Mouse.OverrideCursor = System.Windows.Input.Cursors.Arrow;
}

Care to elaborate, as mentioned, I'm new to C# and programming in general. — Adephx, Oct 23 '18 at 12:44
Possible duplicate of [How to use string variable in sql statement](https://stackoverflow.com/questions/7263893/how-to-use-string-variable-in-sql-statement) — Fildor, Oct 23 '18 at 12:47
@Fildor I'm using a different code structure, without the cmd. attachment. I tried adding the `Parameter.Add("@Search", search.Text)` under the CommandText and replacing `search.Text` with `(@)Search`, but can't get it to work. It's probably logical to someone experience, but not to a newbie. — Adephx, Oct 23 '18 at 12:53
Yes, you should most definitely worry about this. It's very easy to prevent and is an extremely open hole for anyone otherwise. I'll not say never often, but _never_ use unclean values in SQL statements. Use parameters (or better yet in many cases: Entity Framework can handle most of the SQL plumbing for you) — jleach, Oct 23 '18 at 12:56
@Adephx what in particular was unclear to you? Could that answer be improved in any way, so it would have been more useful? — Fildor, Oct 23 '18 at 13:17
@Fildor I had an issue with the cmd. prefix, as I wasn't using it in my code anywhere and simply adding Parameter.Add under CommandText doesn't work. I'll figure it out eventually. — Adephx, Oct 23 '18 at 13:57

MindSwipe · Answer 1 · 2019-08-09T09:48:11.953

Short answer, yes, you always need to worry about SQL Injection, you wouldn't believe how creative some hackers get with those.
Although I am quite experienced in C# I have never worked with direct SQL in C#, I've always used EF with LINQ for Databases
Anyways here goes:

using (var con = new SqlClient.SqlConnection(conectionString))
using (var cmd = SqlClient.SqlCommand())
{
    con.Open();
    cmd.Connection = con;
    cmd.CommandType = CommandType.Text;
    cmd.CommandText = "SELECT * FROM [cbu_naslovi] WHERE [ADDRESS] LIKE '%@Title%' COLLATE Latin1_general_CI_AI";
    cmd.Parameters.Add("@Title", search.Text);
}

Some Explanation:
using: the using keyword is quite simple. Basically anything that you put in between parentheses (e.g Class c = new Class();) needs to implement IDisposable, this variable gets disposed after the Curly Braces
Syntatic Candy: normally a using() needs to be followed up by curly braces { }, except if it is a single statement or another using statement
Sql Stuff: con.Open opens our Connection to the database, and we set this as the Connection our command will use with cmd.Connection = con.
Now we need to say what type of Command it is. By doing cmd.CommandType = CommandType.Text we tell the Command that it is a SQL Query.
And now we need to actually define our Query, but instead of concating the string with + we use @SomeName as a placeholder.
And finally we replace our @SomeName with our actual values by doing cmd.Parameters.Add("@SomeName", value) where @SomeName can be anything but it needs to be same as the placeholder we want to replace in the SQL Query
This is where we actually protect ourselves against SQL Injection attacks, as the cmd.Parameters.Add() does everything we need would otherwise need to do manually

Hope this helps and doesn't confuse you any more

Note: if you are interested in Entity Framework (EF) or LINQ here is a good source: EF Database First tutorial

Could you elaborate the difference between having a parameter '%@Text%' and including the value directly - '%" + search.Text + "%'? I always assumed that single quotes essentially lock the value to text only, so filling the Search `TextBox` with something as "BILLY AND TRUNCATE TABLE TABLE_NAME" shouldn't be possible? How does that type of an injection works? — Adephx, Oct 23 '18 at 14:04
With the `'%" + search.Text + "%'` approach an attacker can just simply put a `'` in front of his string to finish the quotation marks — MindSwipe, Oct 23 '18 at 14:07

score 3 · Accepted Answer · answered Oct 23 '18 at 12:53

3

You are vulnerable to SQL Injection.

First, it is better to enumerate what you want in your select like this:

SELECT Name, Address, City FROM [YourTable] instead of SELECT * FROM [YourTable]

Second, you should insert your parameter in your query like this:

SELECT Name, Address, City FROM [YourTable] WHERE Name = @Name

And in your command:

cmd.Parameters.AddWithValue("@Name", YourValue)

Hope it helps,

Here's some links: What are good ways to prevent SQL injection? https://www.codeproject.com/Tips/706692/Preventing-SQL-Injection-Attacks

answered Oct 23 '18 at 12:53

Olivier Belanger

180
1
8

Should I enumerate even when I want to select every column, because I'm selecting about 12 columns? – Adephx Oct 23 '18 at 13:03
1

Yes. It's almost always a bad idea to `select *`. @Adephx – Fildor Oct 23 '18 at 13:13
Selecting named fields rather than "Select *" does nothing to prevent SQL injection. – Peregrine Oct 23 '18 at 13:21
@Peregrine The "Don't select * - Rule" is not for preventing SQL-Injection. This has other implications. Just add a Column and all your statements could potentially be broken ... (just to name one). – Fildor Oct 23 '18 at 13:23
I never said that "Select *" was good practice, but the question and this answer are specifically about SQL injection – Peregrine Oct 23 '18 at 13:24
@Peregrine I was referring to your comment _"Selecting named fields rather than "Select *" does nothing to prevent SQL injection."_ OP's question about selecting specific columns was not related to SQL Injection (see above comments). I did not mean to claim you said anything about it being good practice. – Fildor Oct 23 '18 at 13:58

WPF/SQL: Replacing values with parameters?

2 Answers2