Security: Cross-Site Request Forgery Protection

[dereks]

RFM is currently vulnerable to this attack:

https://en.wikipedia.org/wiki/Cross-site_request_forgery

I will fix this by using the Cookie-to-Header Token method, which uses a cookie and header called X-Csrf-Token. (Note that JQuery already has known strategies for including this header in all JSON requests.)

As part of this task, I will also audit the source code for this:

https://en.wikipedia.org/wiki/Cross-site_scripting

...by walking through this list of checks:

https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet

I believe these practices are already followed in RFM, but I will try to do a more formal audit.

Thanks!

[joeaudette]

In the api section it mentions requestParams

requestParams - Default value []. Parameters which will be passed with each request to the connector.

couldn't that be used to pass xsrf token to the server?

[psolom]

requestParams was initially designed to pass parameters to server-side connector.

There was a discussion about this, however participants did not reach an agreement, so this param exists, but not in use.

@LegoStormtroopr this issue was raised by @dereks and I hope that he will be back one day to perform the audit of source code as he initially intended.

[joeaudette]

@servocoder requestParams still exists and works for passing extra params right? so it could be used for this purpose right? I am asking for myself, whether that solution satisfies others is up to them, but it seems I could solve it that way.

Different server side frameworks have different solutions for xsrf anyway so there is probably not a one size fits all solution. In asp.net core we typically pass the token as a hidden form field, other stacks may use request headers. For my purposes in asp.net core it seems like the requestParams would make it possible for me to pass the token I need.

[psolom]

requestParams exists but does NOT work, it's just a stub.

You could try to edit filemanger.js file and add token to the requests where it's supposed to be passed (I believe it is must for all POST requests). If this will solve your problem you may get back and we discuss the way of leverage requestParams param.

NOTE: some requests are performed as GET, but logically they should be changed to POST (copy / move files etc.). This could be also discussed within this issue. Keep this in mind and let me know your opinion after the investigation.

[joeaudette]

I agree requests that make changes or do operations should all be post not get.

I wish requestParams did work, I think that would be sufficient solution, and more general purpose since there may be other needs for extra params or multiple extra params.

I don't want to use a fork, I would rather use your standard implementation. I could possibly help and make a pull request but it may be a little while before I could get to it. I have other things I'm working on right now but have a project in my future pipeline where I will need something like RFM and so far it looks best among the similar things I have been evaluating. I saw the recent connector for asp.net core, it is a good start but could use some improvements, especially for xsrf. Also it cannot be used by just dropping in the connector, it has to be compiled into an app or as a dll. Ultimately I plan to make a nuget package which is how we distribute .net components and I will include RFM as embedded resources so people can just plugin the nuget and it works.

[psolom]

Ok, I have created request_updates branch. It's designed to pass params to server for specific request method of for any. See code snippet below. Parameters specified in "GET" object will affect only GET requests. The same is true for "POST". "MIXED" parameters will affect both request types: GET & POST.

Test it and give the feedback please.

    "api": {
        "lang": "php",
        "connectorUrl": false,
        "requestParams": {
            "GET": {
                "getParam": "foo"
            },
            "POST": {
                "postParam": "bar"
            },
            "MIXED": {
                "mixedParam": "mixed"
            }
        }
    },

NOTE: I haven't changed request type from GET to POST for methods that I mentioned in the prev message yet. I will do this the next step, once the current stage is approved. So at this point there are only 3 POST requests: "upload", "savefile" and "extract". The others are GET reqests.

[psolom]

I saw the recent connector for asp.net core, it is a good start but could use some improvements, especially for xsrf. Also it cannot be used by just dropping in the connector, it has to be compiled into an app or as a dll. Ultimately I plan to make a nuget package which is how we distribute .net components and I will include RFM as embedded resources so people can just plugin the nuget and it works.

I support the client-side and PHP connector solely. Other connectors were created by contributors which are listed here. Many of them created connectors for their specific needs, so that these connectors may not support all existing features or configuration options. The most up-to-date connector is PHP, so you can use it as the reference. You can create new issue and try to involve a developer/maintainer in discussion if you wish.

If you are going to improve any connector I will be highly appreciate if you with share it by creating a PR.

[psolom]

Ok, this may be the case.

The problem with this approach is that it assume to pass csrf token via cookie, which is not a appropriate for every framework. Some frameworks support csrf token along with sending form data - POST. Taking this into acount I believe the capability to specify CSRF token in requestParams is more flexible.

Also your script assumes to pass CSRF token in headers, but as far as I remember, there could be solutions which expect to receive CSRF token as request parameter. We need to make deeper investigation on this.

Of course you can go ahead and inject this code into RFM script. However I would like to go further in looking for general-purpose solution. Any ideas are appreciated.

[joeaudette]

I agree, cookie seems a not great idea for this. If I understand how xsrf attacks work, they take advantage of the fact that a user is logged into some site ie the user has an authentication cookie. So the user is logged into siteA with privileged access. Then they trick the user by phishing to visit siteB, and in siteB they make a form that posts to siteA, since the user has a cookie for siteA that gets submitted with the form and the user is tricked into performing some action on siteA. Seems to me that if the xsrf token is also in a cookie then that would also get passed to siteA and therefore the attack could succeed and the xsrf token is not providing protection. xsrf token should be passed either by a form field or by a header other than cookies I would think.

[dereks]

Hi all! I'm the issue reporter. I apologize I haven't worked on this yet. I'm supposed to be dropping to part-time on my current contract, so hopefully in the next one or two weeks I'll be able to start contributions again.

... if the xsrf token is also in a cookie then that would also get passed to siteA and therefore the attack could succeed and the xsrf token is not providing protection

No, that's not how XSRF Cookie-to-Header protection works.

While it is true that the cookie will get passed to the server in your example (like any other cookie from that domain), the server does not use it. Instead, the XSRF protection is based entirely on the fact that the cookie is submitted as an HTTP Header. The attacking "siteB" will not be able to read the cookie in order to copy it into the HTTP Headers (with Javascript), due to the browser's "same-origin" policy.

From Wikipidia:

Security of this technique is based on the assumption that only JavaScript running within the same origin will be able to read the cookie's value. JavaScript running from a rogue file or email will not be able to read it and copy into the custom header. Even though the csrf-token cookie will be automatically sent with the rogue request, the server will be still expecting a valid X-Csrf-Token header.

I proposed this solution because RFM is (basically) entirely AJAX, which works transparently with the Cookie-to-Header protection. For example, there is no POST form submitted when you click to delete or move a file -- the request is submitted as a JSON request instead -- so the Synchronizer token pattern (which you seem to be proposing) would not work here. (Also, Synchronizer token pattern breaks other things, too. See Wikipedia.)

Also your script assumes to pass CSRF token in headers, but as far as I remember, there could be solutions which expect to receive CSRF token as request parameter.

No, the CSRF token does nothing for security if it is a request parameter. Trying to use a Synchronizer token instead -- which does work as a request parameter -- would mean a major rewrite of the RFM protocol. It would also break a bunch of UI things, since the server must generate a unique token for every time data is submitted (instead of a single token that lasts for the entire session).

Please, please, please don't try to invent a home-brew solution to security issues.

Regarding the Cookie-to-Header method, it is a publicly vetted and well-tested solution to this problem, and it would have no negative effects on the UI:

This technique is implemented by many modern frameworks, such as Django[23] and AngularJS.[24] Because the token remains constant over the whole user session, it works well with AJAX applications, but does not enforce sequence of events in the web application.

Thanks!

[joeaudette]

There are definitely solutions that do use hidden form fields aka request parameters. Cookies is one solution, but it has some drawbacks, ie it is passed with every request, even for static resources like css and js.
https://stackoverflow.com/questions/20504846/why-is-it-common-to-put-csrf-prevention-tokens-in-cookies

In asp.net core for example the hidden form field is created for you automatically by default inside any form element in a razor view and then is validated in the controller actions using a filter declared as an attribute on the controller action like this:

[HttpPost]
[Authorize(Policy = "FileManagerPolicy")]
[ValidateAntiForgeryToken] 
public async Task<IActionResult> Upload(
    bool? resizeImages,
    int? maxWidth,
    int? maxHeight,
    string currentDir = "",
    string croppedFileName = ""
    )
{
     ...
 }

[joeaudette]

not to say I'm against the cookie to header idea, as long as we get a solution that works, but I am also not sure the header name used is the same in every server framework

[dereks]

There are definitely solutions that do use hidden form fields aka request parameters.

True, that is called the Synchronizer token pattern. I did mention that (above), and I gave the reasons it would not work well for RFM.

The primary reason is that a new token must be generated for every time data is submitted. That works well for HTML forms, where the form (including the hidden form field holding the token) is sent to the browser before the user submits any data. But it does not work well for an event-based system like RFM, where an AJAX request is submitted ad hoc. Major new code would need to be written to use Synchronizer tokens in RFM.

Cookies is one solution, but it has some drawbacks, ie it is passed with every request, even for static resources like css and js.

This does not make sense to me.

First, there is no need to pass the CSRF Header token back as a cookie after the initial transfer -- it can be deleted by client-side Javascript, if you're really worried about those extra ~30 bytes of network traffic.

Second, normally one would want to protect all server content, including css and js files. Simple knowing what version of a web app is installed on the server (by reading a .js file) is a security vulnerability.

But finally, if you don't care about a particular URL, there is no need to pass the CSRF Header token for those resources. You'd just need to write special code to not protect certain paths.

I am also not sure the header name used is the same in every server framework

Not, the header name is not the same everywhere, just as the Synchronizer token name (in forms) is not the same everywhere. But the name X-Csrf-Token is a popular convention, and probably more standard than any form-based token name.

[psolom]

Welcome back @dereks

I agree that request-based CSRF token is a case for HTML forms. Whatever, I know I can completely rely on you in terms of security and hope you will find time for this soon.

[psolom]

@LegoStormtroopr, please don't delete your messages. As you could see your post resulted in the interesting discussion, and it's possible that your code snippet will be used as a base for the final implementation eventually. I restore your original post for the reference:

I found a solution for this (which is mostly Django specific (also I'm hoping to have a nifty Django connector for you soonly - this library is amaaaazing!)

The only thing I needed to do, was follow the instuctions from Django around getting the CSRF token for AJAX requests. When added to the filemanager.init.js, the CSRF issue went away entirely:

$('.fm-container').richFilemanager({
    // configuration options not included in .json configuration file, see:
    // https://github.com/servocoder/RichFilemanager/wiki/Configuration-options#client-side-configuration
});

// using jQuery
function getCookie(name) {
    var cookieValue = null;
    if (document.cookie && document.cookie !== '') {
        var cookies = document.cookie.split(';');
        for (var i = 0; i < cookies.length; i++) {
            var cookie = jQuery.trim(cookies[i]);
            // Does this cookie string begin with the name we want?
            if (cookie.substring(0, name.length + 1) === (name + '=')) {
                cookieValue = decodeURIComponent(cookie.substring(name.length + 1));
                break;
            }
        }
    }
    return cookieValue;
}
var csrftoken = getCookie('csrftoken');

function csrfSafeMethod(method) {
    // these HTTP methods do not require CSRF protection
    return (/^(GET|HEAD|OPTIONS|TRACE)$/.test(method));
}
$.ajaxSetup({
    beforeSend: function(xhr, settings) {
        if (!csrfSafeMethod(settings.type) && !this.crossDomain) {
            xhr.setRequestHeader("X-CSRFToken", csrftoken);
        }
    }
});

This might not help in every case, but it seems that it shouldn't overburden RichFilemanager. In fact, it might be best to direct users to the documentation for their own server library (eg. Django, Flask, PHP, etc...) on how to get the token and insert into Jquery.

[bio90]

I'm sorry if anyone proposed this solution earlier but I did not read all posts.
You can solve the CSRF issue JQuery Cookie ( https://github.com/carhartl/jquery-cookie )
You can probably use other libraries that manage your cookies as well.

What I did was using the beforeSend in jQuery before the initialization of RichFileManager:

$(function() {
	$.ajaxSetup({
		beforeSend: function(xhr){
			xhr.setRequestHeader('X-CSRFToken', $.cookie("csrftoken"));
		}
	});
    $('.fm-container').richFilemanager({
		baseUrl: "path/to/RichFileManager"
	});
});

Hope this helps.