The CSRF token requirement introduced in #2014573: D6 fix of csrf security advisory SA-CONTRIB-2013-051 and discussion of the services D6 situation has caused some significant problems for situations where the token needs to be passed in via JavaScript. In the Ooyala module, we have a progress bar that indicates how far along the video upload has progressed. However because this status callback is provided via services (by a POST request), we get caught by the CSRF token not being present.

Not all JS within Drupal (or external libraries) are capable of passing headers to their various JavaScript AJAX requests. For some reason when the CSRF token was first introduced, it was done exclusively through checking HTTP headers, which makes doing things like using Drupal's progress bar pinging a Services-provided callback impossible.

To alleviate this problem, I suggest that we check not only $_SERVER['HTTP_X_CSRF_TOKEN'] but also $_GET['services_token'] (or something) that can be passed directly via the URL. I can't think of any reason why using a query string would be any less secure than using a separate HTTP header, since ultimately the URL itself is just a header in the request.

CommentFileSizeAuthor
#2 services_csrf_easy-2066547.patch951 bytesquicksketch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

Here's the definition of progress.js for example:

Drupal.progressBar = function (id, updateCallback, method, errorCallback) {

We're only given options for updateCallback as a URL. We can't also pass in specific headers. We have a similar problem with the external Ooyala uploader library we're using. Combined, we pretty much have no way of doing uploads (with or without progress bars) because Service's CSRF checking blocks all the requests and we don't have any way of sending that information with the current API.

quicksketch’s picture

Status: Active » Needs review
FileSize
951 bytes

Patch with suggested fix.

Status: Needs review » Needs work

The last submitted patch, services_csrf_easy-2066547.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updating to use correct original issue.

kylebrowning’s picture

Issue summary: View changes

You now no longer need to patch services module Nate :P

kylebrowning’s picture

Sorry this took so long.

kylebrowning’s picture

Status: Needs work » Closed (fixed)