Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Redirect to a page with endResponse to true VS CompleteRequest and security thread

Base on this questions and the answers there, I like to ask what is the proper way of redirecting.

The default way using the Redirect(url, endResponse) is throw the ThreadAbortException because is called with endResponse=true that calls the End() method and so, if you use it inside a try/catch block, this exception shown there and that can be assumed as error, but actually a user is try to redirect to a page by stopping the rest of the page processing.

The other possible ways is to call the Redirect(url, endResponse) with endResponse=false following by HttpContext.Current.ApplicationInstance.CompleteRequest(); By using that you do not get any exceptions.

So the question is what is better to use and why.

like image 326
Aristos Avatar asked Feb 01 '13 06:02

Aristos


2 Answers

You must call the redirect always with endRespose=true or else any hacker can see whats on the page by simple hold the redirect.

To prove that I use the NoRedirect plugin for Firefox to hold the redirect. Then I test the two cases and here is the results:

I have a simple page with that text inside it

<form id="form1" runat="server">
<div>
I am making a redirect - you must NOT see this text.
</div>
</form>

and then on Page load try to make the redirect with both cases:

First case, using the Complete Request();

    try
    {
        // redirect with false that did not throw exception
        Response.Redirect("SecondEndPage.aspx", false);
        // complete the Request
        HttpContext.Current.ApplicationInstance.CompleteRequest();
    }
    catch (Exception x)
    {

    }

and there boom, you can see whats inside the page !

And second case

try
{
    // this is throw the ThreadAbortException exception
    Response.Redirect("SecondEndPage.aspx", true);
}
catch (ThreadAbortException)
{
    // ignore it because we know that come from the redirect
}
catch (Exception x)
{

}

Nothing shown now.

So if you do not like a hacker to see whats on your page, you must call it with endResponse to true and stop what other processing is made -eg return from function and not continue.

If for example you check if the user is authenticated he can see that page or if not he must redirect to login, and even in the login if you try to redirect him with endResponse to false, then holding the redirect the hacker can see - what you believe that can not because you use the Redirect.

My basic point here is to show the security thread that exist if you are not stop to send data back to the browser. The redirect is a header and instruction to the browser, but at the same time you need to stop send any other data, you must stop send any other part of your page.

like image 91
Aristos Avatar answered Oct 18 '22 14:10

Aristos


It is not required to call Response.Redirect with true for endResponse to solve the security issue of outputting the page content after the redirect call. You can accomplish this another way and avoid causing a ThreadAbortException at the same time (which is always bad). Below are snippets of a page I created with 5 buttons that cause redirects in different ways, the RedirectRenderOverride button being the ideal as it is the one that triggers the Render method to do nothing. This has been tested with the NoRedirect add-in. Only two cases avoid outputting anything other than the 302 object moved response - RedirectEnd and RedirectRenderOverride.

Code In Front

<asp:Button ID="Button1" runat="server" OnClick="RedirectCompleteRequest" Text="RedirectCompleteRequest"/>
<asp:Button ID="Button2" runat="server" OnClick="RedirectClear" Text="RedirectClear"/>
<asp:Button ID="Button3" runat="server" OnClick="RedirectRenderOverride" Text="RedirectRenderOverride"/>
<asp:Button ID="Button4" runat="server" OnClick="RedirectEnd" Text="RedirectEnd"/>
<asp:Button ID="Button5" runat="server" OnClick="RedirectEndInTryCatch" Text="RedirectEndInTryCatch"/>

Code Behind

public partial class _Default : Page {
    private bool _isTerminating;

    protected void RedirectEnd(object sender, EventArgs e) { Response.Redirect("Redirected.aspx"); }

    protected void RedirectCompleteRequest(object sender, EventArgs e)
    {
        Response.Redirect("Redirected.aspx", false);
        HttpContext.Current.ApplicationInstance.CompleteRequest();
    }

    protected void RedirectClear(object sender, EventArgs e)
    {
        Response.Clear();
        Response.Redirect("Redirected.aspx", false);
    }

    protected void RedirectRenderOverride(object sender, EventArgs e)
    {
        Response.Redirect("Redirected.aspx", false);
        _isTerminating = true;
    }

    protected void RedirectEndInTryCatch(object sender, EventArgs e)
    {
        try {
            Response.Redirect("Redirected.aspx");
        } catch (ThreadAbortException) {
            // eat it
        } finally {
            Response.Write("Still doing stuff!");
        }
    }

    protected override void RaisePostBackEvent(IPostBackEventHandler sourceControl, string eventArgument)
    {
        if (!_isTerminating) {
            base.RaisePostBackEvent(sourceControl, eventArgument);
        }
    }

    protected override void Render(HtmlTextWriter writer)
    {
        if (!_isTerminating) {
            base.Render(writer);
        }
    }
}

Response.End calls Thread.CurrentThread.Abort internally and, according to Eric Lippert, calling Thread.Abort, "is at best indicative of bad design, possibly unreliable, and extremely dangerous."

like image 26
Sumo Avatar answered Oct 18 '22 15:10

Sumo