SQL Injection

Whats the problem?

I changed the code a little bit:

        public System.Data.DataSet GetDataSet(string sql, string[] szArray)
        {
            using (SqlConnection oConn = new SqlConnection(cnEForm))
            {
                //SqlDataAdapter dataAdapter = new SqlDataAdapter(sql, oConn);
                SqlDataAdapter dataAdapter = new SqlDataAdapter("SELECT * FROM Users  
                     WHERE Firm = @val1 AND LoginID = @val2", oConn);
                DataSet ds = new DataSet();

                // Parameterization of the query string
                if (null != szArray)
                {
                    int arrayLength = szArray.Length;

                   for (int ii = 0; ii < arrayLength; ++ii)
                    {
                        string S = szArray[ii];

                        dataAdapter.SelectCommand.Parameters.Add("@val" + (ii + 1).ToString(),
                           SqlDbType.VarChar, S.Length).Value = S;
                    }
                }

                dataAdapter.Fill(ds);

                return ds;
            }
        }

Now, if I put the "SELECT statement" inside the dataAdapter then the sql injection fixed. Unfortunately, I cannot hard code the "SQL statement". Do you all have any idea?

Make sure that S.Length is a reasonable value.

The way you are doing it, you are setting the length of the parameter to the data that is passed. Anything can go in.

Here is the updated code:

        public System.Data.DataSet GetDataSet(string sql, string[] szArray)
        {
            using (SqlConnection oConn = new SqlConnection(cnEForm))
            {
                //SqlDataAdapter dataAdapter = new SqlDataAdapter(sql, oConn);
                SqlDataAdapter dataAdapter = new SqlDataAdapter("SELECT * FROM Users  
                     WHERE Firm = @val1 AND LoginID = @val2", oConn);
                DataSet ds = new DataSet();

                // Parameterization of the query string
                if (null != szArray)
                {
                    int arrayLength = szArray.Length;

                   for (int ii = 0; ii < arrayLength; ++ii)
                    {
                       dataAdapter.SelectCommand.Parameters.Add("@val" + (ii + 1).ToString(),
                           SqlDbType.VarChar).Value =  szArray[ii];
                    }
                }

                dataAdapter.Fill(ds);

                return ds;
            }
        }

Now, if I put the "SELECT statement" inside the dataAdapter then the sql injection fixed. Unfortunately, I cannot hard code the "SQL statement". Do you all have any idea?

What is telling you that?  It may be because it can't examine the SQL statements when they are not hard coded.

The idea is not to remove the Length. You are not solving SQL injection problems by doing so. It is to make sure that it is not too big.

As an example, if you do not expect any piece Firm to be over 40 characters and the LoginID to be over 10 characters , then check the lengths before setting them in the parameter definitions. If any of them gets bigger than the limit you have set, then either stop the application with a SecurityException or truncate it to number of characters you decided upon.