ansaurus

Question

Are any of these SQL Queries open to SQL injection attacks?

Answer 1

+2 A:

Seems fine to me.

Basically, if you don't concatenate SQL string and uses parametrized queries, you're safe against SQL injection attacks.

Rubens Farias 2010-01-29 10:29:44

Answer 2

+2 A:

You're using SqlParameters which effectively removes all SQL injection issues.

Chris S 2010-01-29 10:30:04

Answer 3

+4 A:

It seems you are safe from SQL injection attacks, but code like this:

Response.Write(result);

and:

Response.Write("<b><u> --- Begin SQL Exception Message ---</u></b><br />")
Response.Write(ex)
Response.Write("<br /><b><u> --- End SQL Exception Message ---</u></b>")

could leave you open for other forms of attack such as XSS. You should set the text element of an ASP.NET control, not directly write to the page.

Mark Byers 2010-01-29 10:37:55

Thanks mark i'll implement that now

Phil 2010-01-29 10:40:40

Regardless of it being bad practice to perform `Response.Write` in ASP.NET, how is this open to XSS? None of that what's being written to the page is user input. XSS (if it was there) doesn't go away just because you set the `Text` property on say an ASP.NET label. You should use the AntiXSS library if it is user input that you are displaying on the page.

Wim Hollebrandse 2010-01-29 10:41:25

Thanks for the extra information Wim. For best practice sake would;Catch ex As SqlException SQLErrLabel.Text = ex.ToStringbe more appropriate?

Phil 2010-01-29 10:45:07

Yeah that would be fine, although you may not want to show the complete stacktrace to the end user, but simply display the exception message like `SQLErrLabel.Text = ex.Message;` should suffice. Or alternatively wrap it with a generic error message and log the specifics of the exception appropriately for you to inspect.

Wim Hollebrandse 2010-01-29 10:47:54

@Wim: Well maybe not in this case. Hopefully the exception will never include any user data in the error message, but do you want the security of your system to rely on that always being true? But my main point is that using `Response.Write` in general is an unnecessary security risk. It might be that it's safe here, but why take the risk when there is a perfectly good solution without the risk?

Mark Byers 2010-01-29 10:51:45

@Mark I'll reiterate what I posted earlier, `Response.Write` is no more a 'security risk' than setting the same information (whatever it is) on the `Text` property of an ASP.NET label control.

Wim Hollebrandse 2010-01-29 11:11:07

You both make good points. Thanks a lot for the interest

Phil 2010-01-29 11:11:40

Yes, sadly the `Label.Text` property takes HTML markup *not* text as the name might lead you to believe — making it just as dangerous as `Response.Write`. To use either of these methods you must use `HTMLEncode` on the text value first. It is highly unfortunate that ASP.NET bungles this one, especially given that other classes like TextBox have an apparently-identical `Text` property that doesn't take HTML.

bobince 2010-01-29 11:41:28

Worse, the warning about it on the MSDN doc implies it isn't a problem by saying that “by default, ASP.NET Web pages validate that user input does not include script or HTML elements”. This is totally disingenuous, a dangerous misrepresentation of how much the utterly bogus “XSS protection” in ASP.NET protects you (tip: not at all, and it messes up your web pages in the process too).

bobince 2010-01-29 11:42:15

Answer 4

+2 A:

You can run the static code analysis tool CAT.NET to identify all XSS and SQL injection vectors accross a project, including referenced assemblies.

http://www.microsoft.com/downloads/details.aspx?FamilyId=0178e2ef-9da8-445e-9348-c93f24cc9f9d&displaylang=en

Reports usually make for some interesting reading.

Matt 2010-01-29 11:08:28

Answer 5

+1 A:

You should run a scanner to check for potential SQL injection vulnerabilities. I have had some luck with http://sqlmap.sourceforge.net/

Daniel Coffman 2010-01-29 19:14:16

ansaurus

tags:

views:

answers:

Are any of these SQL Queries open to SQL injection attacks?

related questions