Reading Preventing SQL Injection Attacks in Stored Procedures started this debate between my partner and I---and looking for the right approach is not as straight forward.
I'm no security expert--but what do a person do when you take a look at the first few lines of this stored procedure....and you see this.
We haven't had any significant security issues, but I don't won't to assume that this approach is working...If so leave it alone or make it better.
ASP:
Dim conStr = ConfigurationSettings.AppSettings("WebUser")
Dim command = ("Execute dbo.web_Insert_usr_Comments @Location," & _
"@userexp, @full_name, @emailAddr, @comments")
Using con As New SqlConnection(conStr)
Using cmd As New SqlCommand(command, con)
cmd.Parameters.AddWithValue("@Location", Request.QueryString("Location"))
cmd.Parameters.AddWithValue("@userexp", Request.QueryString("usrexp"))
cmd.Parameters.AddWithValue("@comments", Request.QueryString("comments"))
cmd.Parameters.AddWithValue("@full_name", Request.QueryString("full_name"))
cmd.Parameters.AddWithValue("@emailAddr", Request.QueryString("emailAddr"))
con.Open()
cmd.ExecuteNonQuery()
End Using
End Using
TSQL:
ALTER PROCEDURE [dbo].[web_Insert_usr_Comments]
@Location varchar(255),
@usrexp int = NULL,
@full_name varchar(255) = NULL,
@emailAddr varchar(320) = NULL,
@comments varchar(125) = NULL
AS
BEGIN
DECLARE @securityChk varchar(255), @haveContact INT
SET @securityChk = (SELECT @location + CAST(@usrexp AS VARCHAR(2)) + @full_name + @emailAddr + @usr_comments )
IF @securityChk LIKE '%SELECT%' RETURN;
IF @securityChk LIKE '%DROP%' RETURN;
IF @securityChk LIKE '%INSERT%' RETURN;
IF @securityChk LIKE '%DELETE%' RETURN;
IF @securityChk LIKE '%EXE%' RETURN;
SET @haveContact = ( SELECT (count(*))
FROM dbo.contacts
WHERE ( @emailAddr = E_mail_address AND @location = company))
--> Check and see if we have this person contact info
IF ( @haveContact = 0 )
BEGIN
INSERT INTO dbo.contacts(e_mail_address, company, nick_name)
VALUES (@emailAddr, @location, @full_name)
END
ELSE
BEGIN
INSERT INTO web_comments_dtl(timeGMT, form_id, contact_id, comment, user_exp_ranking, IP_Addr)
SELECT GETDATE()
, 1 --> TODO: Need better logic for form_id
, ( SELECT MAX(id)
FROM dbo.contacts
WHERE ( @emailAddr = E_mail_address AND @location = company)
OR ( @emailAddr = E_mail_address AND @full_name = nick_name )
OR ( @location = company))
, @RFXcomments
, @usrexp
, dbo.fnBinaryIPv4(@ip_Addr)
END
END
What I would like to do, but not sure which option is the most efficient.
- Should spend a week or two and investigate the security issues (if there is one) and the user input form that calls this SP
- Rewrite the whole thing including the SP, code behind, and reconfigure IIS
- Add more clauses to the statement above.
I don't think there is a security problem...but if the guy before decides to put this in his code--what do a young DBA is supposed to do.
@securityChk
into a string andEXEC
-ing this? If so that won't be sufficient to prevent SQL injection. Just use parameterised queries. Also what if my surname is "wardrop"? Will this stop me registering for your service? – Martin Smith Dec 23 '13 at 17:15INSERT INTO T VALUES (@p1,@p2,@p3)
- as long as the rest of the app also uses parameterised queries correctly. – Martin Smith Dec 23 '13 at 17:34