Preventing SQL Injection by cancelling KeyPress events

[thugster69]

Hey guys,

I recently discovered that my SQL statement is prone to simple SQL injections like typing " ';" or " ' " without quotations..

My first remedy for this was to include a REGEX.replace statement to the code:

Dim [B]strUsernameRegex[/B] As String = Regex.Replace(txtUsername.Text, "[^0-9a-zA-Z ]+?", "")
        Dim[B] strPasswordRegex[/B] As String = Regex.Replace(txtPassword.Text, "[^0-9a-zA-Z ]+?", "")
        Dim sqlcomm As New SqlClient.SqlCommand( _
            "USE master_db" & vbCrLf & _
            "SELECT * FROM [users_table] WHERE username='" & [B]strUsernameRegex[/B] & "' AND password='" & [B]strPasswordRegex[/B] & "';", sqlconn)

But, having a lot of Textboxes, to which a user can type the said Injection, It will be a waste of code-space and time to code all of them with a single REGEX.REPLAY each..

I'm searching for a more efficient way on handling this error.. I hope you can help me on this.

Thanks!

 

[cjard]

Why don't you just use a parameterized query? In the programming terms youre familiar with, what youre essentially doing right now is: Having a chunk of VB code stored in a string, replace parts of it with variable text, write it to disk as a file, launch the compiler to compile it into an exe, and then run the compiled exe.. It sounds crazy, right? Noone would write an app like that, just to accept the input of the user's age - they jsut store the age into a variable already cmpiled into the program, rather than writing a fileto disk that contains "Dim age As Integer = 50"
So why go this route with the SQL? (It's exactly what is happening)

Parameterized queries are immune to SQL injection attacks, perform better and promote good practices such as code reuse; there are no downsides and they should always be used if applicable

Dim sqlcomm As New SqlClient.SqlCommand( _
            "USE master_db" & vbCrLf & _
            "SELECT * FROM [users_table] WHERE username= @u AND password= @p;", sqlconn)
sqlcomm.Parameters.AddWithValue("@u", txtUsername.Text)
sqlcomm.Parameters.AddWithValue("@p", txtPassword.Text)

No rocket surgery required

 

[thugster69]

@cjard
I will be trying that now.. Looks cool and much compact.. It almost seems like a Stored Procedure.. Anyways that this proposition will lead to that?

Thanks for your time and concern on this one!

Cheers mate!


EDIT:

I've tried this and it worked.. I apologize for the long code but here it is:

 Dim sqlcomm As New SqlCommand( _
"INSERT INTO student_list VALUES(@stdno,@lastName,@firstName,@middleName,@choice, @date, @placeofBirth, @address,@telno, @mobile,@religion,@fatherName, @occuFather, @contactFather, @motherName, @occuMother, @contactMother)", sqlconn)
        sqlcomm.Parameters.AddWithValue("@stdno", txtStudentNumber.Text)
        sqlcomm.Parameters.AddWithValue("@lastname", txtLastname.Text)
        sqlcomm.Parameters.AddWithValue("@firstName", txtFirstname.Text)
        sqlcomm.Parameters.AddWithValue("@middleName", txtMiddleName.Text)
        sqlcomm.Parameters.AddWithValue("@choice", choice.ToString)
        sqlcomm.Parameters.AddWithValue("@date", shortDate.ToString)
        sqlcomm.Parameters.AddWithValue("@placeofBirth", txtPlaceOfBirth.Text)
        sqlcomm.Parameters.AddWithValue("@address", txtHomeAddress.Text)
        sqlcomm.Parameters.AddWithValue("@telno", mskTelNo.Text)
        sqlcomm.Parameters.AddWithValue("@mobile", mskMobile.Text)
        sqlcomm.Parameters.AddWithValue("@religion", cmbReligion.Text)
        sqlcomm.Parameters.AddWithValue("@fatherName", txtFather.Text)
        sqlcomm.Parameters.AddWithValue("@occuFather", txtOccuFather.Text)
        sqlcomm.Parameters.AddWithValue("@contactFather", txtContactFather.Text)
        sqlcomm.Parameters.AddWithValue("@motherName", txtMother.Text)
        sqlcomm.Parameters.AddWithValue("@occuMother", txtOccuMother.Text)
        sqlcomm.Parameters.AddWithValue("@contactMother", txtContactMother.Text)

From this one:

        'Dim strSQL As String
        'strSQL = "USE master_db" & vbCrLf & " "
        'strSQL = "INSERT INTO student_list VALUES('" & txtStudentNumber.Text & "', '" & txtLastname.Text & "'"
        'strSQL += ", '" & txtFirstname.Text & "', '" & txtMiddleName.Text & "', '" & choice.ToString & "' "
        'strSQL += ", '" & shortDate.ToString & "', '" & txtPlaceOfBirth.Text & "', '" & txtHomeAddress.Text & "'"
        'strSQL += ", '" & mskTelNo.Text & "', '" & mskMobile.Text & "', '" & cmbReligion.Text & "'"
        'strSQL += ", '" & txtFather.Text & "', '" & txtOccuFather.Text & "', '" & txtContactFather.Text & "'"
        'strSQL += ", '" & txtMother.Text & "', '" & txtOccuMother.Text & "', '" & txtContactMother.Text & "')"

It's longer but the code is much leaner now right?

 

[JuggaloBrotha]

@cjard
I will be trying that now.. Looks cool and much compact.. It almost seems like a Stored Procedure.. Anyways that this proposition will lead to that?

Thanks for your time and concern on this one!

Cheers mate!

You can work with queries, views and stored proc's the same with that type of code cjard posted. You can even throw everything into a dataset file (*.xsd), define the views and their related queries all with the visual designer making the Parameterized queries for you. Then you just Dim YourVar As New DSNameTableAdapters.TheDefinedTable
YourVar.NameOfQueryYouGaveIt(arg1, arg2, etc..)

 

[thugster69]

@JuggaloBrotha

I had a hard time understanding your post.. If you may elaborate more?

 

[JuggaloBrotha]

Right click your project (in the Solution Explorer) and click Add -> New item. Select 'DataSet', give it a name & click 'Ok'. Now you have an empty dataset designer, right click anywhere and click Add -> TableAdapter. The first window of the wizard lets you select (or specify) a connection to a database (the connection string) then the rest lets you build the query like Access and SQL Server do (you can link tables, do joins etc.. as well as use a Query/View or a Stored Proc or just a table) and you specify the column(s) and the inputs (if any are needed, use the Filter column) then you give it a name. You'll see a table object in the desginer now, that's the DataTable (you use that in code like any other DataTable) and below it is that query you made (the query name is the method call you can use in code). You can add other queries to it by right clicking it and selecting 'Add Query' and run through the wizard again.

Then in code you simply make a variable of the TableAdapter and can call those queries (methods) by name, pass in any parameters (if any) and if you need to pass a DataTable, just dim a DataTable of the type that goes with that TableAdapter.

 

[thugster69]

@Juggalo

It's a good idea of you, I tried it to generate an SQL SELECT command and it worked.. But with SQL UPDATE, I get this parsing error when i click the Query Builder

Error in table name or view name in UPDATE clause.
Unable to parse query text.

 

[JuggaloBrotha]

@Juggalo

It's a good idea of you, I tried it to generate an SQL SELECT command and it worked.. But with SQL UPDATE, I get this parsing error when i click the Query Builder

Error in table name or view name in UPDATE clause.
Unable to parse query text.

So what's your update query? Odds are you're missing set placeholder(s). If your query looks similar to this:
Set Column1 =, Column2 =, Column3 =
Then all you have to do is fill in the place placeholders so it's:
Set Column1 = ?, Column2 = ?, Column3 = ? for MS Access or Set Column1 = @Col1, Column2 = @Col2, Column3 = @Col3 for everything else.

Alternatively you could use the QueryBuilder and specify the ?'s or @'s in the filter column for those fields.

 

[thugster69]

I see.. I'll be using it.. I guess I just haven't get the hang of it yet.. If I may ask another question, but it's out of topic:

How can I use the value selected from a combobox as a field in an SQL statement?

Mine is this:

 Dim sqlcommSearch As New SqlCommand("SELECT * FROM [student_list] WHERE @criteria = @keyword", sqlconn)
        sqlcommSearch.Parameters.AddWithValue("@criteria", cmbCriteria.SelectedItem.ToString)
        sqlcommSearch.Parameters.AddWithValue("@keyword", txtKeyword.Text)

but it returns no result..

 

[cjard]

@criteria is a column name. You cannot parameterize column names, kinda like you can't do this:

Dim controlType as New String("Button")

DIm myButton as New contolType 'WILL NOT MAKE A NEW BUTTON

If you want to offer a parameterized query that picks from a combo box either:

Dim sqlcommSearch As New SqlCommand("SELECT * FROM [student_list] WHERE " & combo.Text & " = @keyword", sqlconn)

MAKE SURE the user cannot type into the combo box. Actually this is even still dangerous because the text can be altered via windows API calls, DLL injection, hexediting the EXE (possibly) but the chances are small.. This is also an option:

Dim sqlcommSearch As New SqlCommand("SELECT * FROM [student_list] WHERE (@criteria = 'name' AND name_column = @keyword) OR (@criteria = 'age' AND age_column = @keyword)", sqlconn)

If you choose "name" then @criteria parameter gets filled with "name", and your query predicate becomes:

("name" = "name" and name = 'John') OR ("name" = "age" AND age = 'John')

The one thing to watch here is in datatype conversion. The db may choose to convert all million age records to string to do the comparison, or it may complain that you put a string (john) where a number was expected

Of the two, I'd go with the first, and make sure the user can't edit the text

 

[cjard]

JuggaloBrotha is talking about using the built in facilities in .net to write the code youre doing now

If you want to read and write the customer table, youy add a new TableAdapter and give it a query of SELECT * FROM customer WHERE name = @name (for example)

Visual Studio will then work out the Insert, Update and Delete queries (you need a primary key on the table for U and D)

You see a local reprepsentation of the table that you can manipulate:

myTableAdapter.FillByName(customerTable, "John")
customerTable(0).Age = 22
myTableAdapter.Update(customerTable) 'update means 'save' - it processes all pending Insert/Update/Delete


Thats how we edit data in the modern world

We also connect textboxes to the customerDatatable so that we dont have to push the .Text values around

For more info follow the tutorial in my signature, DW3. Read the Creating a Simple Data App one

 

[thugster69]

@cjard

@criteria is a column name. You cannot parameterize column names, kinda like you can't do this:

Dim controlType as New String("Button")

DIm myButton as New contolType 'WILL NOT MAKE A NEW BUTTON

======>what do you mean in this?

If you want to offer a parameterized query that picks from a combo box either:

Dim sqlcommSearch As New SqlCommand("SELECT * FROM [student_list] WHERE " & combo.Text & " = @keyword", sqlconn)

===> this is when I need to pick values from a combobox as the keyword right?

Dim sqlcommSearch As New SqlCommand("SELECT * FROM [student_list] WHERE (@criteria = 'name' AND name_column = @keyword) OR (@criteria = 'age' AND age_column = @keyword)", sqlconn)

If you choose "name" then @criteria parameter gets filled with "name", and your query predicate becomes:

("name" = "name" and name = 'John') OR ("name" = "age" AND age = 'John')

===>This is where I get confused.. It's a bit blurry in this line:

(@criteria = 'name' AND name_column = @keyword) OR (@criteria = 'age' AND age_column = @keyword)

The one thing to watch here is in datatype conversion. The db may choose to convert all million age records to string to do the comparison, or it may complain that you put a string (john) where a number was expected

==the code works when the keyword is a number and post an error when a string is queried.

Of the two, I'd go with the first, and make sure the user can't edit the text

What first?

Thanks for caring in my problem.. It has been troubling me for so long.. I wish someday I can give back to all of your help..

EDIT:

I'll be back here in the forums tomorrow, I gotta sleep and rest a bit since lately I've been lacking some sleep due to programming..

Thanks cjard!

 

[cjard]

@cjard


======>what do you mean in this?

I mean you can store the name of a Type (Button, String, Form, File, WebClient.. basically anything you can make a New one of) intoa string, but you can't then create a new object of that type from the string

i.e. you Cannot make a program that will let the user type "DataGridView" in a text box, and then they click a button and lo and behold a datagridview appears underneath the button


For this same reason you cannot say:

SELECT * FROM @tablename


And then populate the @tablename parameter with "people" or "customer" or "order" etc... If you cannot understand why at this point in your programming career, then just learn that the only thing you can put in a parameter is a VALUE, not a column name, not a table name, and not a query

===> this is when I need to pick values from a combobox as the keyword right?

No, this is when you need to choose which column from the table you want to search for keyword in

e.g. your form looks like:

Search in column [COMBO_BOX_OF_COLUMN_NAMES] for value [TEXTBOX_THAT_USER_TYPES_VALUE_INTO]

"SELECT * FROM table WHERE " & combo.Text & " = @keyword"
Parameters.AddWithValue("@keyword", textbox.Text)

===>This is where I get confused.. It's a bit blurry in this line:

(@criteria = 'name' AND name_column = @keyword) OR (@criteria = 'age' AND age_column = @keyword)

Do the logic in your head. From the above, suppose the combo contains "name" and "age" and you have selected "name"

Parameters.AddWithValue("@criteria", combo.text) 'combo has "name" selected
Parameters.AddWithValue("@keyword", textbox.text) 'textbox has JOHN written in it

SQL is effectively:

([COLOR="silver"]@criteria[/COLOR]'name' = 'name' AND name_column = [COLOR="silver"]@keyword[/COLOR]'JOHN') [B]THIS TEST IS TRUE IF THE name_column CONTAINS JOHN[/B]
OR 
([COLOR="silver"]@criteria[/COLOR]'name' = 'age' AND age_column = [COLOR="silver"]@keyword[/COLOR]'JOHN') [B]THIS TEST IS ALWAYS ALWAYS ALWAYS FALSE NO MATTER WHAT IN THE AGE COLUMN![/B]

==the code works when the keyword is a number and post an error when a string is queried.

Probably because the database is attempting to convert a non-numerical string to a number. You have to be careful/take extra steps when mixing data types. You'd be better off using the other query as it's simpler

 

[thugster69]

@cjard

Thanks for replying to my problem! Now I'm using the whole Parametization in all my SQL statement.. It cancels the possibility of an SQL Injection without the technical fuzz..

Thanks for this! I really appreciate it!