0

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.

Community
  • 1
  • 1
Rick Putnam
  • 546
  • 1
  • 6
  • 20

3 Answers3

1

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.

Lazarus
  • 41,906
  • 4
  • 43
  • 54
  • CAT.NET appears to generate a lot of false positives along these lines. I was looking for confirmation that I wasn't missing anything and I appreciate your comment on that. – Rick Putnam Jan 05 '11 at 21:08
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.

Robb
  • 3,801
  • 1
  • 34
  • 37
0

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.

Jamie Treworgy
  • 23,934
  • 8
  • 76
  • 119
  • Well, yes, that's what I was asking -- given the success of parsing this input to an integer -- is this code genuinely vulnerable. My suspicion was no and that seems confirmed by comments here for which I'm grateful. Clearly, simply stuffing the contents of txtInput into the Url is not safe. – Rick Putnam Jan 05 '11 at 21:06
  • Actually my point is that simply stuffing the contents of txtInput into a URL is *not* unsafe, since anyone can stuff anything they want into a URL that hits your web server. It may result in ugly situations, but it's no more unsafe than the existence of myPage.aspx in the first place. It's what the page at the other end of the URL does that makes it safe or not. – Jamie Treworgy Jan 05 '11 at 21:20