From a6253486c04de4ffbc8a2017d669015137f6f423 Mon Sep 17 00:00:00 2001
From: Gigi <140321494+gigioneggiando@users.noreply.github.com>
Date: Sat, 26 Sep 2026 18:02:22 +0300
Subject: [PATCH] Require POST+CSRF for self-update mutations; guard
 post-config step (CWE-352) (#9335)

* update: require POST + CSRF for updater mutations; guard post-config step

The self-update check/apply mutations were reachable via GET, which is never
anti-CSRF validated (the token is only checked on POST), and the privileged
post-configuration step (do_post_update()/PostUpdate hook, removal of the
staged update file) was additionally reauth-exempt and GET-reachable (CWE-352).

- checkAction and the apply branch now require POST, so the global anti-CSRF
  token check applies; the update page's Check/Apply controls are POST forms
  carrying the token.
- The post_conf finalisation is bound to a one-time session marker set by the
  successful apply right before its internal redirect, so it only runs as that
  continuation, not via a direct cross-site GET (independent of reauth config).
- Reauth is now required consistently, including for apply/post_conf (was
  exempted); the legitimate post_conf redirect still passes on the reauth done
  moments earlier by the apply request.

Status/index endpoints remain GET and read-only.

* Reduce some comments

---------

Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
---
 app/Controllers/updateController.php | 27 +++++++++++++++++++++------
 app/views/update/index.phtml         | 12 +++++++++---
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/app/Controllers/updateController.php b/app/Controllers/updateController.php
index 15d83235b..ecd5aacb2 100644
--- a/app/Controllers/updateController.php
+++ b/app/Controllers/updateController.php
@@ -133,8 +133,7 @@ class FreshRSS_update_Controller extends FreshRSS_ActionController {
 			Minz_Error::error(403);
 		}
 
-		if (!(Minz_Request::actionName() === 'apply' && Minz_Request::paramBoolean('post_conf')) &&
-			FreshRSS_Auth::requestReauth()) {
+		if (FreshRSS_Auth::requestReauth()) {
 			return;
 		}
 
@@ -187,6 +186,11 @@ class FreshRSS_update_Controller extends FreshRSS_ActionController {
 		Else via system configuration  auto_update_url
 	*/
 	public function checkAction(): void {
+		if (!Minz_Request::isPost()) {
+			Minz_Request::forward(['c' => 'update', 'a' => 'index'], true);
+			return;
+		}
+
 		FreshRSS_View::prependTitle(_t('admin.update.title') . ' · ');
 		$this->view->_path('update/index.phtml');
 
@@ -298,6 +302,12 @@ class FreshRSS_update_Controller extends FreshRSS_ActionController {
 		}
 
 		if (Minz_Request::paramBoolean('post_conf')) {
+			if (!Minz_Session::paramBoolean('update_post_conf_ok')) {
+				Minz_Request::forward(['c' => 'update', 'a' => 'index'], true);
+				return;
+			}
+			Minz_Session::_param('update_post_conf_ok', false);
+
 			if (self::isGit()) {
 				$res = !self::hasGitUpdate();
 			} else {
@@ -321,16 +331,19 @@ class FreshRSS_update_Controller extends FreshRSS_ActionController {
 				Minz_Request::bad(_t('feedback.update.error', is_string($res) ? $res : 'unknown'), [ 'c' => 'update', 'a' => 'index' ]);
 			}
 		} else {
+			if (!Minz_Request::isPost()) {
+				Minz_Request::forward(['c' => 'update', 'a' => 'index'], true);
+				return;
+			}
+
 			$res = false;
 
 			if (self::isGit()) {
 				$res = self::gitPull();
 			} else {
 				require UPDATE_FILENAME;
-				if (Minz_Request::isPost()) {
-					// @phpstan-ignore function.notFound
-					save_info_update();
-				}
+				// @phpstan-ignore function.notFound
+				save_info_update();
 				// @phpstan-ignore function.notFound
 				if (!need_info_update()) {
 					// @phpstan-ignore function.notFound
@@ -345,6 +358,8 @@ class FreshRSS_update_Controller extends FreshRSS_ActionController {
 			}
 
 			if ($res === true) {
+				// Authorise the single internal post-configuration redirect that follows.
+				Minz_Session::_param('update_post_conf_ok', true);
 				Minz_Request::forward([
 					'c' => 'update',
 					'a' => 'apply',
diff --git a/app/views/update/index.phtml b/app/views/update/index.phtml
index 3bebb2a79..92fad9027 100644
--- a/app/views/update/index.phtml
+++ b/app/views/update/index.phtml
@@ -61,7 +61,10 @@
 	?>
 	<div class="form-group form-actions">
 		<div class="group-controls">
-			<a href="<?= _url('update', 'check') ?>" class="btn btn-important"><?= _t('admin.update.check') ?></a>
+			<form method="post" action="<?= _url('update', 'check') ?>">
+				<input type="hidden" name="_csrf" value="<?= FreshRSS_Auth::csrfToken() ?>" />
+				<button type="submit" class="btn btn-important"><?= _t('admin.update.check') ?></button>
+			</form>
 		</div>
 	</div>
 	<?php } ?>
@@ -69,8 +72,11 @@
 	<?php if ($this->update_to_apply) { ?>
 	<div class="form-group form-actions">
 		<div class="group-controls">
-			<a class="btn btn-attention btn-state1" href="<?= _url('update', 'apply') ?>" data-state2-id="button-update-loading"><?= _t('admin.update.apply') ?></a>
-			<span class="btn btn-state2" id="button-update-loading"><?= _t('admin.update.loading') ?></span>
+			<form method="post" action="<?= _url('update', 'apply') ?>">
+				<input type="hidden" name="_csrf" value="<?= FreshRSS_Auth::csrfToken() ?>" />
+				<button type="submit" class="btn btn-attention btn-state1" data-state2-id="button-update-loading"><?= _t('admin.update.apply') ?></button>
+				<span class="btn btn-state2" id="button-update-loading"><?= _t('admin.update.loading') ?></span>
+			</form>
 		</div>
 	</div>
 	<?php } ?>