McAfee Vulnerability ??

[Mike718NY]

McAfee took the seal off my website because they say I have a Vulnerability.
The page they pointed out produced this:

[[ConLib:Custom/ProdList]] Unclosed quotation mark after the character string ';",)` AND VisibilityId=0 ORDER BY name ASC'.

I'm guessing it's from this line of code from a custom page I made:

ProductCollection productList = ProductDataSource.LoadForCriteria(
"ManufacturerId=" + Request.QueryString["ID"] + " AND VisibilityId=0", "name ASC");

Is this a real threat or just one falsely flagged by McAfee?

[Logan Rhodehamel]

Very real threat. What you are doing is insecure. You are passing unfiltered querystring data into a sql query.

Code: Select all

ManufacturerId=" + AlwaysConvert.ToInt(Request.QueryString["ID"]) + " AND VisibilityId=0", "name ASC");

Do that instead, ASAP.

[Mike718NY]

Thanks
Originally I was thinking along the same lines for a fix, like doing something like this:

int ID = (int)Request.QueryString["ID"];
.......

but your way does the trick.

[Logan Rhodehamel]

Thanks
Originally I was thinking along the same lines for a fix, like doing something like this:

int ID = (int)Request.QueryString["ID"];
.......

but your way does the trick.

I think your code may cause a crash if it can't cast the value into integer.. any time you take input from the query string assume someone is going to try to put bad data into it. The always convert function attempts a conversion to int. If it can't convert it to int, it returns a 0 without a crash.

[Mike718NY]

I'm getting the message:

The name 'AlwaysConvert' does not exist in the current context

Do I need to reference another AC assembly?
All I have now is:

using CommerceBuilder.Products;

[Logan Rhodehamel]

Either CommerceBuilder.Utility or CommerceBuilder.Common... can't remember which off hand.

[Mike718NY]

CommerceBuilder.Utility
thanks