CAT.NET: vulnerability or false positive?

Question

2nd in an occasional series: Here's the first one

Is CAT.NET correct that the following is a genuine vulnerability in ASP.NET or is it a false positive?

var myInt = Int32.Parse(txtUserInput.Text);

Response.Redirect(string.Format("myPage.aspx?myId={0}", myInt);

CAT.NET is reporting this as a redirect vulnerability needing remediation via encoding myInt.

Answer 1

I wouldn't call that dangerous but its not how I would write it myself

int myInt;
if(Int32.TryParse(txtUserInput.Text,out myInt)){
    Response.Redirect(string.Format("myPage.aspx?myId={0}", myInt);
   }

Is to my mind cleaner as it wont throw an exception if the parse fails due to bad user input and we are explicitly typing the int.

Any error handling code can be bundled into an else statement on the end.

Answer 2

I don't believe so, it could cause an exception so TryParse might be a better approach. It's just yelling because you are taking user input and redirecting based on it. It's possibly being a little too aggressive which isn't exactly bad.

Answer 3

There is no exploitable vulnerability as a result of this code. Any vulnerability would be a result of what myPage.aspx does with the value of myId, not how your url is built. Anyone could just as easily directly hit myPage.aspx with anything they want in the querystring.

However this is bad practice, assuming that you haven't left anything out of the code between those two lines. You should verify that txtUserInput.Text contains only numeric characters, and falls within allowable values.

Exploits happen because of improper parsing of user-supplied data by the page it's posted to -- not improper generating of URLs. While it's a good idea to try to make sure your web site won't write a broken URL because of something that's put in a form, input validation at the front-end is irrelevant to security. All that matters is what the code that accepts the input does with it, since any post or query string can be forged.