Received in my email today:
say your blog and thought you might help.
strsql = “SELECT StaffID, DesignationID, StaffName, Password, ShopID from staffT where StaffName =” & UserName.Text & ” AND Password =” & Password.Text & “”
from the string, the username.text and password.text are form controls. what is happening is there are passing null values regardless of what you input in the text boxes resulting in a system error.
“System Error Object reference not set to an instance of an object”
Am using Mysql as the database.
I’m always glad to answer such questions, especially when the questioner is flirting with disaster, as much as this questioner is.
A trained eye can immediately spot the problem with the SQL statement above, aside from the problem of NULL values tossing errors. Namely, it’s wide-open to SQL injection. (And an even keener eye will note that the values for user name and password aren’t delimited with single-quotes.)
So here’s my reply email to the questioner:
Your SQL statement has three problems.
- It is wide open to injection attack. See http://www.unixwiz.net/techtips/sql-injection.html for examples.
- As you noted, when nulls are passed in, the expression fails.
- It appears you have not delimited your text inputs with single quotes.
Assuming you are using ASP.NET, and that your user names and passwords are only alphanumeric, the direct fix to your problem is this:
Dim strUser As String If String.IsNullOrEmpty(UserName.Text) Then strUser = String.Empty Else strUser = UserName.Text.Replace("'", "''") End If Dim strPass As String If String.IsNullOrEmpty(Password.Text) Then strPass = String.Empty Else strPass = Password.Text.Replace("'", "''") End If strsql = "SELECT StaffID, DesignationID, StaffName, Password, ShopID from staffT where StaffName = '" & strUser & "' AND Password = '" & strPass & "'"
That should get you going, but you should employ the following best practices fixes:
- Use stored procedures with paramerterized input values. This serves to automatically sanitize your queries for data type.
- Use RequiredFieldValidators and RegularExpressionValidators to ensure that you receive some sort of input for each field, and that each does not contain an effort to inject your code with SQL.
- Impose some sort of control that limits multiple login attempts.
- Better yet, use the built-in membership provider in ASP.NET, which can be used with MySQL, and has already resolved many potential attack vectors.
Hope this helps.
Additional Thoughts And Clarification
The guidance I gave the emailer, instructing him to escape single quotes, is a bare-minimum escaping sequence. As a practical matter, he should sanitize his inputs against additional MySQL sequences, such as double-dashes and semicolons, as well as reserved SQL statement words, such as DROP, ALTER, DELETE, etc. There’s an HTTPModule example over at the ASP.NET forums that does this automatically for an application.
In addition to securing data type, query parameters limit the ability of an attacker to inject SQL by fixing the form of the query implicitly. In other words, it’s harder for him to mangle a parameter than it is to mangle a string.
All links in this post on delicious: http://delicious.com/stacks/view/Kp3pOU