Cross-site request forgery/Confused deputy problem

This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.
9 years ago
And here we go. Done. Please see changeset f798ea024d9f.

Thanks again for this contribution!
9 years ago
In the latest checkin a notice a great start to the XSS issues with NOP.

I notice when reviewing the code that there are occasions when the contributor was unable to get the validation working.

This was when reading data from grids when they supported filtering.  There were a couple of comments such as...

//any-forgery does not work with this contentType for some reasons
//data: addAntiForgeryToken


I played around for a little bit trying various methods to submit the token but I too could not get it to work.  I would always get warned that the token was not present.

Eventually I did get it working by combining a few tricks I found in various forums.

You can submit the token as a header using the beforeSend configuration option of a dataSource.

read: {
    url: "@Html.Raw(Url.Action("AllSettings", "Setting"))",
    type: "POST",
    dataType: "json",
    contentType: "application/json",
    beforeSend : function (req) {
        var token = $('[name=__RequestVerificationToken]').val();
        req.setRequestHeader('__RequestVerificationToken', token);
    }
},


This grabs the token value and creates a request header with the same name and submits it as part of the request.

Out of the box the ValidateAntiForgeryTokenAttribute does not look for the token in the headers collection but instead looks in the form body.

This is easy to fix as we now have the AdminAntiForgeryAttribute already created.

If you change the OnAuthorization method to the following...

public virtual void OnAuthorization(AuthorizationContext filterContext)
{
    if (filterContext == null)
        throw new ArgumentNullException("filterContext");

    if (_ignore)
        return;

    //don't apply filter to child methods
    if (filterContext.IsChildAction)
        return;

    //only POST requests
    if (!String.Equals(filterContext.HttpContext.Request.HttpMethod, "POST", StringComparison.OrdinalIgnoreCase))
        return;

    if (!DataSettingsHelper.DatabaseIsInstalled())
        return;
    var securitySettings = EngineContext.Current.Resolve<SecuritySettings>();
    if (!securitySettings.EnableXsrfProtectionForAdminArea)
        return;

    var httpContext = filterContext.HttpContext;
    if (httpContext.Request.Headers["__RequestVerificationToken"] != null)
    {
        var cookie = httpContext.Request.Cookies[AntiForgeryConfig.CookieName];
        AntiForgery.Validate(cookie != null ? cookie.Value : null, httpContext.Request.Headers["__RequestVerificationToken"]);
        return;
    }

    var validator = new ValidateAntiForgeryTokenAttribute();
    validator.OnAuthorization(filterContext);

}


We now look for the existence of the token in the header and if it is there we validate it.  If not we just carry on and validate in the normal way.

You should also remove the
[AdminAntiForgery(true)] 
from the controller.

This should mean it is possible to validate everything in the future.

PS: Headers are also encrypted if the site is SSL protected so this is just as secure as when the token is in the form.

Let me know what you think.
9 years ago
Great. Thanks a lot for this contribution. I've just created a work item (will have a look at it a bit later).

BTW, we can ignore it now because there's no any real issue. A malicious person cannot use it to hack a store because it simply loads some data.
9 years ago
Yeah I get that.  It was more for completeness than anything else.
8 years ago
Hello,

It looks like this fix is not part of the nopCommerce_3.50_Source.

Please let me know.

Thank you very much,
Dmitry
This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.