2

I've written the below code to redirect to another page in my website

 if (!string.IsNullOrEmpty(Request.QueryString["id"]))
   Response.Redirect("node.aspx?id=" + Request.QueryString["id"], false);

This code is working without any issues but fortify on demand does show the open redirect vulnerability in the above code.
Can anyone please help me on this ?

Daniel A. White
  • 187,200
  • 47
  • 362
  • 445
user3089816
  • 191
  • 1
  • 8
  • 21
  • 1
    Side note: while I understand that this is sample code and you've removed all encoding to simplify it, please make sure your real code correctly constructs url and don't use string concatenation unless you are string escape master (see http://stackoverflow.com/questions/14517798/append-values-to-query-string if you forgot how to construct urls) – Alexei Levenkov Feb 23 '17 at 17:50
  • please see this https://learn.microsoft.com/en-us/aspnet/mvc/overview/security/preventing-open-redirection-attacks – RohitS Feb 23 '17 at 17:55
  • You can refer to this question http://stackoverflow.com/questions/8574764/how-to-prevent-open-redirection-attacks. Though it doesn't directly relate to your question but it shows one primary step to prevent open redirect vulnerability that is checking if it's a local URL or not. – kazisami Feb 23 '17 at 19:39

1 Answers1

1

I'm not familiar with fortify on demand. open redirect vulnerability basically is to redirect a user to the URL which is passed in QueryString. Normally, we see that in Login page, but it is not the case in your code.

If you'd know that id is an integer value, you could parse it first.

int id;
string str = Request.QueryString["id"];
if (!string.IsNullOrEmpty(str) && Int32.TryParse(str, out id))
    Response.Redirect("node.aspx?id=" + id, false);
zx485
  • 28,498
  • 28
  • 50
  • 59
Win
  • 61,100
  • 13
  • 102
  • 181
  • what changes i need to made if query string value is string ? – user3089816 Feb 23 '17 at 19:12
  • Well, if value is string, all you can do is `if (!string.IsNullOrEmpty(str)){...}`. Validation error basically is a false positive. – Win Feb 23 '17 at 19:43