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.
Comment | File | Size | Author |
---|---|---|---|
#2 | services_csrf_easy-2066547.patch | 951 bytes | quicksketch |
Comments
Comment #1
quicksketchHere's the definition of progress.js for example:
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.
Comment #2
quicksketchPatch with suggested fix.
Comment #3.0
(not verified) CreditAttribution: commentedUpdating to use correct original issue.
Comment #4
kylebrowning CreditAttribution: kylebrowning commentedYou now no longer need to patch services module Nate :P
Comment #5
kylebrowning CreditAttribution: kylebrowning commentedSorry this took so long.
Comment #7
kylebrowning CreditAttribution: kylebrowning commented