Permalink
Browse files

security(actions): don't refresh invalid CSRF tokens in the page

The refresh token process no longer blindly replaces all in-page CSRF tokens,
as these could have been in user content. Only tokens that have been validated
on the server are replaced.

We can validate tokens even if the session had expired because we base the
tokens not on session ID, but rather a unique key stored in the session and
passed to the client.

On token refresh, Elgg now provides a page refresh link and an appropriate
message depending on whether the user was logged out or into another account
(e.g. via another tab).

BREAKING CHANGE:
The JS function `elgg.security.setToken` is now formally marked private and
its parameters are not backwards compatible.
  • Loading branch information...
mrclay committed May 31, 2016
1 parent eaa9c73 commit 9d8ddecb90b9e160ad85610592c5808e7e8f0c3f
@@ -47,7 +47,7 @@ public function execute($action, $forwarder = "") {
if (!in_array($action, $exceptions)) {
// All actions require a token.
- action_gatekeeper($action);
+ $this->gatekeeper($action);
}
$forwarder = str_replace(_elgg_services()->config->getSiteUrl(), "", $forwarder);
@@ -143,12 +143,7 @@ public function validateActionToken($visible_errors = true, $token = null, $ts =
$session_id = _elgg_services()->session->getId();
if (($token) && ($ts) && ($session_id)) {
- // generate token, check with input and forward if invalid
- $required_token = $this->generateActionToken($ts);
-
- // Validate token
- $token_matches = _elgg_services()->crypto->areEqual($token, $required_token);
- if ($token_matches) {
+ if ($this->validateTokenOwnership($token, $ts)) {
if ($this->validateTokenTimestamp($ts)) {
// We have already got this far, so unless anything
// else says something to the contrary we assume we're ok
@@ -256,20 +251,43 @@ public function gatekeeper($action) {
forward(REFERER, 'csrf');
}
+
+ /**
+ * Was the given token generated for the session defined by session_token?
+ *
+ * @param string $token CSRF token
+ * @param int $timestamp Unix time
+ * @param string $session_token Session-specific token
+ *
+ * @return bool
+ * @access private
+ */
+ public function validateTokenOwnership($token, $timestamp, $session_token = '') {
+ $required_token = $this->generateActionToken($timestamp, $session_token);
+
+ return _elgg_services()->crypto->areEqual($token, $required_token);
+ }
/**
+ * Generate a token from a session token (specifying the user), the timestamp, and the site key.
+ *
* @see generate_action_token
+ *
+ * @param int $timestamp Unix timestamp
+ * @param string $session_token Session-specific token
+ *
+ * @return string
* @access private
*/
- public function generateActionToken($timestamp) {
- $session_id = _elgg_services()->session->getId();
- if (!$session_id) {
- return false;
+ public function generateActionToken($timestamp, $session_token = '') {
+ if (!$session_token) {
+ $session_token = elgg_get_session()->get('__elgg_session');
+ if (!$session_token) {
+ return false;
+ }
}
- $session_token = _elgg_services()->session->get('__elgg_session');
-
- return _elgg_services()->crypto->getHmac([(int)$timestamp, $session_id, $session_token], 'md5')
+ return _elgg_services()->crypto->getHmac([(int)$timestamp, $session_token], 'md5')
->getToken();
}
@@ -317,14 +317,15 @@ public function set_ignore_access($ignore = true) {
/**
* Adds a token to the session
*
- * This is used in creation of CSRF token
+ * This is used in creation of CSRF token, and is passed to the client to allow validating tokens
+ * later, even if the PHP session was destroyed.
*
* @return void
*/
protected function generateSessionToken() {
// Generate a simple token that we store server side
if (!$this->has('__elgg_session')) {
- $this->set('__elgg_session', md5(microtime() . rand()));
+ $this->set('__elgg_session', _elgg_services()->crypto->getRandomString(22));
}
}
View
@@ -268,22 +268,40 @@ function ajax_action_hook() {
}
/**
- * Send an updated CSRF token
+ * Send an updated CSRF token, provided the page's current tokens were not fake.
*
* @access private
*/
function _elgg_csrf_token_refresh() {
-
if (!elgg_is_xhr()) {
return false;
}
+ $actions = _elgg_services()->actions;
+
+ // the page's session_token might have expired (not matching __elgg_session in the session), but
+ // we still allow it to be given to validate the tokens in the page.
+ $session_token = get_input('session_token', null, false);
+ $pairs = (array)get_input('pairs', array(), false);
+ $valid_tokens = (object)array();
+ foreach ($pairs as $pair) {
+ list($ts, $token) = explode(',', $pair, 2);
+ if ($actions->validateTokenOwnership($token, $ts, $session_token)) {
+ $valid_tokens->{$token} = true;
+ }
+ }
+
$ts = time();
$token = generate_action_token($ts);
$data = array(
- '__elgg_ts' => $ts,
- '__elgg_token' => $token,
- 'logged_in' => elgg_is_logged_in(),
+ 'token' => array(
+ '__elgg_ts' => $ts,
+ '__elgg_token' => $token,
+ 'logged_in' => elgg_is_logged_in(),
+ ),
+ 'valid_tokens' => $valid_tokens,
+ 'session_token' => elgg_get_session()->get('__elgg_session'),
+ 'user_guid' => elgg_get_logged_in_user_guid(),
);
header("Content-Type: application/json");
View
@@ -3,49 +3,100 @@
*/
elgg.provide('elgg.security.token');
-elgg.security.tokenRefreshFailed = false;
-
elgg.security.tokenRefreshTimer = null;
/**
- * Sets the currently active security token and updates all forms and links on the current page.
+ * Updates in-page CSRF tokens that were validated on the server. Only validated __elgg_token values
+ * are replaced.
*
- * @param {Object} json The json representation of a token containing __elgg_ts and __elgg_token
+ * @param {Object} token_object Value to replace elgg.security.token
+ * @param {Object} valid_tokens Map of valid tokens (as keys) in the current page
* @return {Void}
+ *
+ * @private
*/
-elgg.security.setToken = function(json) {
- //update the convenience object
- elgg.security.token = json;
-
- //also update all forms
- $('[name=__elgg_ts]').val(json.__elgg_ts);
- $('[name=__elgg_token]').val(json.__elgg_token);
+elgg.security.setToken = function(token_object, valid_tokens) {
+ // update the convenience object
+ elgg.security.token = token_object;
+
+ // also update all forms
+ $('[name=__elgg_ts]').val(token_object.__elgg_ts);
+ $('[name=__elgg_token]').each(function () {
+ if (valid_tokens[$(this).val()]) {
+ $(this).val(token_object.__elgg_token);
+ }
+ });
// also update all links that contain tokens and time stamps
$('[href*="__elgg_ts"][href*="__elgg_token"]').each(function() {
- this.href = this.href
- .replace(/__elgg_ts=\d*/, '__elgg_ts=' + json.__elgg_ts)
- .replace(/__elgg_token=[0-9a-f]*/, '__elgg_token=' + json.__elgg_token);
+ var token = this.href.match(/__elgg_token=([0-9a-z_-]+)/i)[1];
+ if (valid_tokens[token]) {
+ this.href = this.href
+ .replace(/__elgg_ts=\d+/i, '__elgg_ts=' + token_object.__elgg_ts)
+ .replace(/__elgg_token=[0-9a-z_-]+/i, '__elgg_token=' + token_object.__elgg_token);
+ }
});
};
/**
* Security tokens time out so we refresh those every so often.
+ *
+ * We don't want to update invalid tokens, so we collect all tokens in the page and send them to
+ * the server to be validated. Those that were valid are replaced in setToken().
*
* @private
*/
elgg.security.refreshToken = function() {
- elgg.getJSON('refresh_token', function(data) {
- if (data && data.__elgg_ts && data.__elgg_token) {
- elgg.security.setToken(data);
- if (elgg.is_logged_in() && data.logged_in === false) {
- elgg.session.user = null;
- elgg.register_error(elgg.echo('session_expired'));
- }
+ // round up token pairs present
+ var pairs = {};
+
+ pairs[elgg.security.token.__elgg_ts + ',' + elgg.security.token.__elgg_token] = 1;
+
+ $('form').each(function () {
+ // we need consider only the last ts/token inputs, as those will be submitted
+ var ts = $('[name=__elgg_ts]:last', this).val();
+ var token = $('[name=__elgg_token]:last', this).val();
+ // some forms won't have tokens
+ if (token) {
+ pairs[ts + ',' + token] = 1;
}
});
-};
+ $('[href*="__elgg_ts"][href*="__elgg_token"]').each(function() {
+ var ts = this.href.match(/__elgg_ts=(\d+)/i)[1];
+ var token = this.href.match(/__elgg_token=([0-9a-z_-]+)/i)[1];
+ pairs[ts + ',' + token] = 1;
+ });
+
+ pairs = $.map(pairs, function(val, key) {
+ return key;
+ });
+
+ elgg.ajax('refresh_token', {
+ data: {
+ pairs: pairs,
+ session_token: elgg.session.token
+ },
+ dataType: 'json',
+ method: 'POST',
+ success: function(data) {
+ if (data) {
+ elgg.session.token = data.session_token;
+ elgg.security.setToken(data.token, data.valid_tokens);
+
+ if (elgg.get_logged_in_user_guid() != data.user_guid) {
+ elgg.session.user = null;
+ if (data.user_guid) {
+ elgg.register_error(elgg.echo('session_changed_user'));
+ } else {
+ elgg.register_error(elgg.echo('session_expired'));
+ }
+ }
+ }
+ },
+ error: function() {}
+ });
+};
/**
* Add elgg action tokens to an object, URL, or query string (with a ?).
@@ -99,6 +150,9 @@ elgg.security.addToken = function(data) {
throw new TypeError("elgg.security.addToken not implemented for " + (typeof data) + "s");
};
+/**
+ * @private
+ */
elgg.security.init = function() {
// elgg.security.interval is set in the js/elgg PHP view.
elgg.security.tokenRefreshTimer = setInterval(elgg.security.refreshToken, elgg.security.interval);
View
@@ -20,7 +20,8 @@
'logout' => "Log out",
'logoutok' => "You have been logged out.",
'logouterror' => "We couldn't log you out. Please try again.",
- 'session_expired' => "Your session has expired. Please reload the page to log in.",
+ 'session_expired' => "Your session has expired. Please <a href='javascript:location.reload(true)'>reload</a> the page to log in.",
+ 'session_changed_user' => "You have been logged in as another user. You should <a href='javascript:location.reload(true)'>reload</a> the page.",
'loggedinrequired' => "You must be logged in to view the requested page.",
'adminrequired' => "You must be an administrator to view the requested page.",
@@ -17,6 +17,7 @@
),
'session' => array(
'user' => null,
+ 'token' => _elgg_services()->session->get('__elgg_session'),
),
);

0 comments on commit 9d8ddec

Please sign in to comment.