Rework CSRF interaction with sessions (#2290)

* Rework CSRF interaction with sessions

Fix https://github.com/FreshRSS/FreshRSS/issues/2288
Improve security in some edge cases
Maybe relevant for
https://github.com/FreshRSS/FreshRSS/issues/2125#issuecomment-474992671

* Forgotten mime type

app/Controllers/authController.php

@@ -69,7 +69,7 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 	 * the user is already connected.
 	 */
 	public function loginAction() {
-		if (FreshRSS_Auth::hasAccess()) {
+		if (FreshRSS_Auth::hasAccess() && Minz_Request::param('u', '') == '') {
 			Minz_Request::forward(array('c' => 'index', 'a' => 'index'), true);
 		}
 
@@ -133,6 +133,7 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 				// Set session parameter to give access to the user.
 				Minz_Session::_param('currentUser', $username);
 				Minz_Session::_param('passwordHash', $conf->passwordHash);
+				Minz_Session::_param('csrf');
 				FreshRSS_Auth::giveAccess();
 
 				// Set cookie parameter if nedded.
@@ -161,6 +162,8 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 				return;
 			}
 
+			FreshRSS_FormAuth::deleteCookie();
+
 			$conf = get_user_configuration($username);
 			if ($conf == null) {
 				return;
@@ -176,6 +179,7 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 			if ($ok) {
 				Minz_Session::_param('currentUser', $username);
 				Minz_Session::_param('passwordHash', $s);
+				Minz_Session::_param('csrf');
 				FreshRSS_Auth::giveAccess();
 
 				Minz_Request::good(_t('feedback.auth.login.success'),

app/Controllers/userController.php

@@ -247,6 +247,7 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 				$user_conf = get_user_configuration($new_user_name);
 				Minz_Session::_param('currentUser', $new_user_name);
 				Minz_Session::_param('passwordHash', $user_conf->passwordHash);
+				Minz_Session::_param('csrf');
 				FreshRSS_Auth::giveAccess();
 			}
 

app/FreshRSS.php

@@ -57,18 +57,26 @@ class FreshRSS extends Minz_FrontController {
 
 	private static function initAuth() {
 		FreshRSS_Auth::init();
-		if (Minz_Request::isPost() && !(is_referer_from_same_domain() && FreshRSS_Auth::isCsrfOk())) {
-			// Basic protection against XSRF attacks
-			FreshRSS_Auth::removeAccess();
-			$http_referer = empty($_SERVER['HTTP_REFERER']) ? '' : $_SERVER['HTTP_REFERER'];
-			Minz_Translate::init('en');	//TODO: Better choice of fallback language
-			Minz_Error::error(
-				403,
-				array('error' => array(
-					_t('feedback.access.denied'),
-					' [HTTP_REFERER=' . htmlspecialchars($http_referer, ENT_NOQUOTES, 'UTF-8') . ']'
-				))
-			);
+		if (Minz_Request::isPost()) {
+			if (!is_referer_from_same_domain()) {
+				// Basic protection against XSRF attacks
+				FreshRSS_Auth::removeAccess();
+				$http_referer = empty($_SERVER['HTTP_REFERER']) ? '' : $_SERVER['HTTP_REFERER'];
+				Minz_Translate::init('en');	//TODO: Better choice of fallback language
+				Minz_Error::error(403, array('error' => array(
+						_t('feedback.access.denied'),
+						' [HTTP_REFERER=' . htmlspecialchars($http_referer, ENT_NOQUOTES, 'UTF-8') . ']'
+					)));
+			}
+			if ((!FreshRSS_Auth::isCsrfOk()) &&
+				(Minz_Request::controllerName() !== 'auth' || Minz_Request::actionName() !== 'login')) {
+				// Token-based protection against XSRF attacks, except for the login form itself
+				Minz_Translate::init('en');	//TODO: Better choice of fallback language
+				Minz_Error::error(403, array('error' => array(
+						_t('feedback.access.denied'),
+						' [CSRF]'
+					)));
+			}
 		}
 	}
 

app/Models/Auth.php

@@ -24,6 +24,7 @@ class FreshRSS_Auth {
 			$conf = Minz_Configuration::get('system');
 			$current_user = $conf->default_user;
 			Minz_Session::_param('currentUser', $current_user);
+			Minz_Session::_param('csrf');
 		}
 
 		if (self::$login_ok) {
@@ -56,6 +57,7 @@ class FreshRSS_Auth {
 				$current_user = trim($credentials[0]);
 				Minz_Session::_param('currentUser', $current_user);
 				Minz_Session::_param('passwordHash', trim($credentials[1]));
+				Minz_Session::_param('csrf');
 			}
 			return $current_user != '';
 		case 'http_auth':
@@ -63,6 +65,7 @@ class FreshRSS_Auth {
 			$login_ok = $current_user != '' && FreshRSS_UserDAO::exists($current_user);
 			if ($login_ok) {
 				Minz_Session::_param('currentUser', $current_user);
+				Minz_Session::_param('csrf');
 			}
 			return $login_ok;
 		case 'none':
@@ -196,13 +199,10 @@ class FreshRSS_Auth {
 	}
 	public static function isCsrfOk($token = null) {
 		$csrf = Minz_Session::param('csrf');
-		if ($csrf == '') {
-			return true;	//Not logged in yet
-		}
 		if ($token === null) {
 			$token = Minz_Request::fetchPOST('_csrf');
 		}
-		return $token === $csrf;
+		return $token != '' && $token === $csrf;
 	}
 }
 

p/scripts/main.js

@@ -44,6 +44,12 @@ var context;
 }());
 //</Global context>
 
+function badAjax() {
+	openNotification(context.i18n.notif_request_failed, 'bad');
+	location.reload();
+	return true;
+}
+
 function needsScroll(elem) {
 	const winBottom = document.documentElement.scrollTop + document.documentElement.clientHeight,
 		elemTop = elem.offsetParent.offsetTop + elem.offsetTop,
@@ -165,6 +171,9 @@ function send_mark_read_queue(queue, asRead) {
 			for (let i = queue.length - 1; i >= 0; i--) {
 				delete pending_entries['flux_' + queue[i]];
 			}
+			if (this.status == 403) {
+				badAjax();
+			}
 		};
 	req.onload = function (e) {
 			if (this.status != 200) {
@@ -269,6 +278,9 @@ function mark_favorite(div) {
 	req.onerror = function (e) {
 			openNotification(context.i18n.notif_request_failed, 'bad');
 			delete pending_entries[div.id];
+			if (this.status == 403) {
+				badAjax();
+			}
 		};
 	req.onload = function (e) {
 			if (this.status != 200) {
@@ -918,6 +930,9 @@ function init_stream(stream) {
 				req.responseType = 'json';
 				req.onerror = function (e) {
 						checkboxTag.checked = !isChecked;
+						if (this.status == 403) {
+							badAjax();
+						}
 					};
 				req.onload = function (e) {
 						if (this.status != 200) {
@@ -1008,6 +1023,9 @@ function updateFeed(feeds, feeds_count) {
 	const req = new XMLHttpRequest();
 	req.open('POST', feed.url, true);
 	req.onloadend = function (e) {
+			if (this.status != 200) {
+				return badAjax();
+			}
 			feed_processed++;
 			const div = document.getElementById('actualizeProgress');
 			div.querySelector('.progress').innerHTML = feed_processed + ' / ' + feeds_count;
@@ -1045,9 +1063,12 @@ function init_actualize() {
 		context.ajax_loading = true;
 
 		const req = new XMLHttpRequest();
-		req.open('GET', './?c=javascript&a=actualize', true);
+		req.open('POST', './?c=javascript&a=actualize', true);
 		req.responseType = 'json';
 		req.onload = function (e) {
+				if (this.status != 200) {
+					return badAjax();
+				}
 				const json = xmlHttpRequestJson(this);
 				if (auto && json.feeds.length < 1) {
 					auto = false;
@@ -1078,7 +1099,10 @@ function init_actualize() {
 					updateFeed(json.feeds, feeds_count);
 				}
 			};
-		req.send();
+		req.setRequestHeader('Content-Type', 'application/json');
+		req.send(JSON.stringify({
+				_csrf: context.csrf,
+			}));
 
 		return false;
 	};