mirror of
https://github.com/FreshRSS/FreshRSS.git
synced 2026-10-28 14:08:45 -04:00
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>
This commit is contained in:
1 parent
6153087f68
commit
a6253486c0
2 files changed
+30
-9
No files matched your search
@@ -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',
|
||||
|
||||
@@ -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 } ?>
|
||||
|
||||
Reference in new issue
Block a user