2

I am making a project in which i have a login page.

i am restricting user to enter

AND OR NOT XOR & | ^

is this enough to prevent my application from SQL Injection?

Javed Akram
  • 14,220
  • 23
  • 77
  • 113
  • Wow, none of the input you are restricting addresses sql injection. Its entirely quote marks and integer casts. – rook Nov 08 '10 at 18:05
  • Haven't you met [Robert '); DROP TABLE Students ;--](http://xkcd.com/327/) yet? ;) – takeshin Nov 08 '10 at 19:01

7 Answers7

12

No, not at all.

For example, I could still enter my username as:

; DELETE FROM Users --

Which could still, depending on your DB structure and application code, wipe your entire Users table.

To adequately protect yourself from SQL Injection attacks you should escape any user input and use either parameterized queries or stored procedures (and if you're using stored procedures, be sure you don't have dynamically generated SQL inside the stored procedure) to interact with the database.

Justin Niessner
  • 229,755
  • 35
  • 391
  • 521
6

You shouldn't bother looking for special words / characters in their username /password. Because you will ALWAYS miss something.

Instead, if you have embedded SQL you should be using parameterized queries. If you do that for all of your queries then you'll be safe from sql injection. Now, XSS is whole other matter.. ;)

This has been covered in depth on this site, just search for sql injection.

NotMe
  • 84,830
  • 27
  • 167
  • 241
2

Using Stored Procedures or parameterized queries will prevent SQL injection.

1) In addition to that, if you are using ASP.NET, you can enable the page level attribute "ValidateRequest = True" which can validate if any of the input string can lead to Script injection

2) Make sure you dont display the actual system generated error to the end user. That will give a lead to the hacker to probe further and break the system.

3) If you are using a webservice to consume and sync the data to your database, validate all the necessary fields before persisting the data.

Hunter
  • 2,212
  • 2
  • 19
  • 22
1

Definitely not!

The simplest possible way to avoid SQL injection is by using parameterized queries.

See this SO question: Preventing SQL Injection in ASP.Net VB.Net and all of its answers to give you an idea.

In short, I never use concatenated string queries, but ALWAYS parameters. This way, there is no danger at all, and this is the most secure way to prevent SQL injection.

Community
  • 1
  • 1
Will Marcouiller
  • 22,531
  • 19
  • 89
  • 142
0

Here is a good stack overflow link: What is SQL injection?

Secondly, don't forget that it doesn't matter what validation you do in the UI, people can always construct custom HTTP requests and send them to your server (trivial as editing using firebug).

Community
  • 1
  • 1
Bradley Kreider
  • 1,093
  • 10
  • 16
0

Like others said. Parameterized inputs.

Here's a snipit from some code I wrote at work(removed work specific code). It's not perfect, but my main job is not programming and I was still researching on C# when I wrote this. If I wrote this now, I would have used a datareader instead of a dataset.

But notice how I use variables in the actual SQL string and assign the variables using "da.SelectCommand.Parameters.AddWithValue"

public Boolean Login(string strUserName, string strPassword)
        {
            SqlConnection sqlConn = new System.Data.SqlClient.SqlConnection();
            DataSet ds = null;
            SqlDataAdapter da = null;

            sqlConn.ConnectionString = strConnString;

            try
            {
                blnError = false;
                sqlConn.Open();

                ds = new DataSet();
                da = new SqlDataAdapter("select iuserid from tbl_Table where vchusername = @vchUserName and vchpassword = @vchPassword", sqlConn);

                da.SelectCommand.Parameters.AddWithValue("@vchUserName", strUserName);
                da.SelectCommand.Parameters.AddWithValue("@vchPassword", strPassword);

                da.SelectCommand.CommandTimeout = 30;
                da.Fill(ds);

                if (ds.Tables[0].Rows.Count > 0)
                {
                    iUserId = (int)ds.Tables[0].Rows[0]["iuserid"];
                }
            }
            catch (Exception ex)
            {
                blnError = true;
                Log("Login: " + ex.Message);
            }
            finally
            {
                if (sqlConn.State != ConnectionState.Closed)
                    sqlConn.Close();
                if (da != null)
                    da.Dispose();
                if (ds != null)
                    ds.Dispose();
            }
            if (blnError)
                return false;
            if (iUserId > 0)
                return true;
            return false;
        }
Bengie
  • 1,025
  • 5
  • 10
-1

You should pass the values as parameters to a stored procedure. This way whatever the user enters is just treated as a value rather than appended to the statement and executed

Karl Gohery
  • 126
  • 1
  • 6