Mysql – Understanding the use of Sql command parameters

ado.netMySQLstored-procedures

I am using MySql database and I have a stored procedure to insert data, like this-

CREATE DEFINER=`root`@`localhost` PROCEDURE `adduser`(
    IN id varchar(10),
    IN pwd varchar(10),
    IN fname varchar(45),
    IN lname varchar(45),
    IN email varchar(45),
    IN phone varchar(13),
    IN address varchar(145),
    IN r1 varchar(45),
    IN r2 varchar(45),
    IN r3 varchar(45),
    IN r4 varchar(45),
    IN r5 varchar(45),
    IN r6 varchar(45),
    IN r7 varchar(45),
    IN r8 varchar(45)
)
BEGIN
    insert into users 
    values(id,pwd,fname,lname,email,phone,address,'yes',r1,r2,r3,r4,r5,r6,r7,r8);
END

In my Asp.net website I have some code like this which calls the stored procedure-

using (cmd = new MySqlCommand("CALL vtsdb.adduser('" + txt_userId.Text + "','" + txt_pwd.Text + "','" + txt_firstName.Text + "','" + txt_lastName.Text + "','" + txt_emailId.Text + "','" + txt_phoneNo.Text + "','" + txt_address.Text + "','" + txt_f1.Text + "','" + txt_f2.Text + "','" + txt_f3.Text + "','" + txt_f4.Text + "','" + txt_f5.Text + "','" + txt_f6.Text + "','" + txt_f7.Text + "','" + txt_f8.Text + "')", con)) 
            {
                cmd.ExecuteNonQuery();
                con.Close();
            }

What I would like to know-

Do I need to use sql parameters in the above code? I dont see my stored procedure vulnerable to a Sql injection attack. Please guide me if I am wrong.

Also what is the use of mentioning CommandType.StoredProcedure in my c# code while my code still executes well?

Best Answer

Your stored procedure, itself, in a vacuum, as written now, does not appear vulnerable to SQL injection... but that is not the only consideration, and is not the final answer.

It's always possible that it could be updated later and become vulnerable, but there's yet something else to consider.

For a much bigger potential problem, take one step back and look at your calling convention.

If someone could manage to pass the following string value to your application as txt_f8.Text... it would be obediently concatenated into your MySqlCommand()...

stuff'); GRANT ALL PRIVILEGES ON *.* TO 'hacker'@'%' IDENTIFIED BY 'pwn3d'; SELECT ('1

...then what would be executed by your server?

Possibly, nothing. Possibly, way too much.

This randomly-googled tutorial reinforces the point of not crafting queries with string concatenation, and using SqlParameter instead:

This situation invites a hacker to replace that string with something malicious. In the worst case, you could give full control of your computer away.

Instead of dynamically building a string, as shown in the bad example above, use parameters. Anything placed into a parameter will be treated as field data, not part of the SQL statement, which makes your application much more secure.

Certainly not a good trade-off for having to do less typing.

I'm a MySQL DBA and not a .NET person, but from what I can tell from a quick search it appears that declaring CommandType.StoredProcedure allows you to take better advantage of the capabilities of stored procedures, by making some of the parameters INOUT or OUT, and not just IN. This might be useful, for example, if you wanted one of the parameters to be the ID of the newly-inserted user, which the SP could return to your code.

If you don't need return values, you don't have to do it that way... but feedback/confirmation isn't usually a bad thing.

Related Question