Evaluate/Refactor my ASP.NET XSS Security Helper Class

I am maintaining a site with significant security concerns and I wrote a helper class for validating potential XSS attacks. The ValidateRequest method is meant to evaluate the HttpRequest for any potential issues, and if issues are found, redirect the user to the same page, but in a away that is not threatening to the application.

The application is being evaluated by HP WebInspect. Their initial review included two regular expressions:

-
The "Regex for a simple XSS attack"

/((\%3C) )/ix

-
The "Paranoid regex for XSS attacks"

/((\%3C) )/I

The regular expression I am using is a more stringent C# adaptation of the paranoid regex. I prefer to err on the side of caution, so I would rather risk denying valid requests than allowing an invalid request to be processed.

Thanks in advance for the help.

```
using System;
using System.Text.RegularExpressions;
using System.Web;

public static class XssSecurity
{

public const string PotentialXssAttackExpression = "(http(s)(%3a|:))|(ftp(s)(%3a|:))|(javascript)|(alert)|(((\\%3C) ))";

private static readonly Regex PotentialXssAttackRegex = new Regex(PotentialXssAttackExpression, RegexOptions.IgnoreCase);

public static bool IsPotentialXssAttack(this HttpRequest request)
{
if(request != null)
{
string query = request.QueryString.ToString();
if(!string.IsNullOrEmpty(query) && PotentialXssAttackRegex.IsMatch(query))
return true;
if(request.HttpMethod.Equals("post", StringComparison.InvariantCultureIgnoreCase))
{
string form = request.Form.ToString();
if (!string.IsNullOrEmpty(form) && PotentialXssAttackRegex.IsMatch(form))
return true;
}
if(request.Cookies.Count > 0)
{
foreach(HttpCookie cookie in request.Cookies)
{
if(PotentialXssAttackRegex.IsMatch(cookie.Value))
{
return true;
}
}

Some general idea:

• Use guard statements instead of big if blocks.

• Extract the !string.IsNullOrEmpty(query) && PotentialXssAttackRegex.IsMatch(query) condition to a method.

• Extract individual checks to distinct methods. The IsPotentialXssAttack will be quite easy to read and the code of different validations will not be mixed in one big method.

• I'd create an expireAllCookies helper method too.

The final code of the first method would look like this (I haven't tested nor compiled):

public static bool IsPotentialXssAttack(this HttpRequest request) {
    if (request == null) {
        return false;
    }
    if (isPotentialXssAttackQuery(request)) {
        return true;
    }
    if (isPotentialXssAttackFormOrAnything(request)) {
        return true;
    }
    if (isPotentialXssAttackCookie(request)) {
        return true;
    }

    return false;
}

public static bool isPotentialXssAttackQuery(this HttpRequest request) {
    string query = request.QueryString.ToString();
    if (isMatch(query)) {
        return true;
    }
    return false;
}

public static bool isMatch(string input) {
    if (string.IsNullOrEmpty(query)) {
        return false;
    }
    return PotentialXssAttackRegex.IsMatch(query);
}

// TODO: rename it
public static bool isPotentialXssAttackFormOrAnything(this HttpRequest request) {
    if (!request.HttpMethod.Equals("post", 
            StringComparison.InvariantCultureIgnoreCase)) {
        return false;
    }

    string form = request.Form.ToString();
    if (isMatch(form)) {
        return true;
    }
    return false;
}

public static bool isPotentialXssAttackCookie(this HttpRequest request) {
    if (request.Cookies.Count <= 0) {
        return false;
    }

    foreach (HttpCookie cookie in request.Cookies){
        if (isMatch(cookie.Value)) {
            return true;
        }
    }
    return false;
}

StackExchange Code Review Q#5988, answer score: 6