After performing countless code reviews for clients I found myself performing the same tasks each time in order to get ramped up on the code and to identify major areas of concern.
When performing a security code review, finding issues like Cross Site Scripting, SQL injection, Poor Input Validation, and others quickly can be a huge time saver. This also helps to make sure that the majority of the code has been reviewed for low hanging fruit, and then later a more in-depth review can be performed of the major areas of concern.
If you've been reading this blog for a while, you may have noticed that I'm a big fan of regular expressions. Some of these issues can be discovered by building a good regular expression. For this purpose I wrote a very basic static analysis tool I've lovingly named YASAT (Yet Another Static Analysis Tool). It uses a list of regular expression rules to scan a source tree and produce a report that a code reviewer could verify. Its purpose is to give you a sense of hot spots and produce many false positives, without too many false negatives so you can use it to start your code review off on the right foot. If you're interested in the tool go check it out on github.
One small caveat: this is not intended to be an exhaustive list of all potential security issues in ASP.NET. There is no replacement for a "brain on, eyes open" line-by-line code review. This is simply intended to give you some good starting points in a new code base quickly.
Cross Site Scripting (XSS)
Look for any Label, Literal, Checkbox, LinkButton, RadioButton or any other control that has a ".Text" property. If the value that is assigned to the .Text is not properly encoded there is a possibility for XSS.
GridViews, DataLists, and Repeaters can be set to either encode by default or not. If you see one of these being used verify that it's being used properly. You set the data on these by assigning the DataSource property to some kind of structured data (usually a DataTable). Make sure the values in the DataSource are properly encoded or that the control is doing this automatically.
Input Validation
.NET can do automatic malicious character detection using the ValidateRequest=true setting. True is the default for this, so if you don't see it it's set properly. You must set this to false if you're going to accept any character that .NET thinks could be dangerous (like < or '), so it's common to see it turned off. This can live either at the top of an aspx file (between the <%@ %> tags) or in the web.config file in the <configuration><system.web><pages validateRequest ="false"> section.
.NET has a bunch of validators (CompareValidator, CustomValidator, RegularExpressionValidator); they all work on the entire string (even if your regular expression lacks the ^$ stuff), however these only check client side by default. (Note: I wrote a blog entry on creating good regular expressions for input validation earlier.) You can check those same validators on the server by checking the Page.IsValid property, but this isn't done by default, so they're probably vulnerable if if you don't see validation and you don't see something like the following:
if(Page.IsValid)
{
//blah
}
Look for Any TextBoxes, DropDownLists, or ListBoxes; the input from all of these should be validated.
SQL Injection
Searching for "using System.Data.SqlClient" will tell you which classes use SQL.
The common ways to execute SQL use the SqlConnection and SqlCommand classes. SqlCommand has ExecuteNonQuery and ExecuteQuery methods. NonQuery returns the number of rows that were affected while ExecuteQuery returns a DataReader used to read the SQL data stream. You'll be able to recognize all kinds of SQL injection possibilities (format strings, concatenations, etc.) when you look at the ExecuteNonQuery and ExecuteQuery methods. If they're using parameterized queries, they're probably fine.
Session
.NET doesn’t do any Cross Site Request Forgery (CSRF) checking by default. If no explicit CSRF token generation and checking functionality is apparent, the code is most likely vulnerable to CSRF.
Cookies are insecure by default. Look for secrets being stored here. Search for anything that says Response.Cookies[“mycookie”] or Request.Cookies.Add. Cookies must be validated from the client and secrets should not be stored in them.
Viewstate is also insecure by default. Look for secrets being stored here too. Viewstate can be encrypted, but this only makes sense if unique secrets are stored here. If you see secrets being stored in viewstate, validate that they are properly protected.
SSL
SSL certificate chains are automatically checked for validity, but often developers will bypass this if using internal self-signed certificates. This SSL Certificate checking can be bypassed by overriding the CheckValidationResult method. If this method always returns true all SSL checking has been bypassed.
Information Disclosure
Make sure exceptions are handled properly. If you see something like Response.Write(ex.ToString()) the exception will be written directly to the client. This can open all kinds of other issues. ToString includes the stack trace and other debugging information. Search for "catch(Exception" to find exception handling code.
The above checklist is a good way to start a code review, but as mentioned earlier, it is not an exhaustive list. No checklist can replace a mature SDL and Code Review process, but hopefully this will give you some high impact issues to quickly check for while doing large scale code reviews.
