mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-23 03:27:02 +09:00
fix(security): [develop] require CSRF-protected POST for link and plugin reordering (#8278)
* security: require CSRF-protected POST for link and plugin reordering (#7454) Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(csrf): share POST and token guards for reorder actions Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
This commit is contained in:
1 parent
ce6ab6e0b3
commit
c749790d84
6 files changed
+93
-6
No files matched your search
+7
-2
@@ -445,11 +445,11 @@ function csrf_error_callback() : void {
|
||||
}
|
||||
|
||||
/**
|
||||
* csrf_require_post - reject a non-POST request for a state-changing action.
|
||||
* csrf_require_post - require a CSRF-protected POST for a state-changing action.
|
||||
*
|
||||
* State-changing actions reached through a direct action= URL must arrive by
|
||||
* POST so they cannot be triggered cross-site with a GET (for example an <img>
|
||||
* tag). csrf-magic validates the token on the resulting POST.
|
||||
* tag). Reject invalid POST tokens before the action can change state.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
@@ -459,4 +459,9 @@ function csrf_require_post() : void {
|
||||
http_response_code(405);
|
||||
exit;
|
||||
}
|
||||
|
||||
if (!csrf_check(false)) {
|
||||
http_response_code(403);
|
||||
exit;
|
||||
}
|
||||
}
|
||||
@@ -68,6 +68,8 @@ switch (grv('action')) {
|
||||
|
||||
break;
|
||||
case 'ajax_dnd':
|
||||
csrf_require_post();
|
||||
|
||||
$new_order = gnrv('dnd');
|
||||
|
||||
links_reorder($new_order);
|
||||
@@ -399,7 +401,8 @@ function pages() : void {
|
||||
$('#dnd').find('tr:first').addClass('nodrag').addClass('nodrop');
|
||||
$('#dnd').tableDnD({
|
||||
onDrop: function(table, row) {
|
||||
loadUrl({url:'links.php?action=ajax_dnd&'+$.tableDnD.serialize()})
|
||||
const request = cactiPreparePostRequest('links.php', 'action=ajax_dnd&' + $.tableDnD.serialize());
|
||||
postUrl({url: request.url}, request.data);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
+4
-1
@@ -408,6 +408,8 @@ switch($action) {
|
||||
|
||||
break;
|
||||
case 'ajax_dnd':
|
||||
csrf_require_post();
|
||||
|
||||
$new_order = gnrv('dnd');
|
||||
|
||||
api_plugin_reorder($new_order);
|
||||
@@ -888,7 +890,8 @@ function update_show_current() : void {
|
||||
// console.log(row);
|
||||
},
|
||||
onDrop: function(table, row) {
|
||||
loadUrl({url:'plugins.php?action=ajax_dnd&'+$.tableDnD.serialize()})
|
||||
const request = cactiPreparePostRequest('plugins.php', 'action=ajax_dnd&' + $.tableDnD.serialize());
|
||||
postUrl({url: request.url}, request.data);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
<?php
|
||||
// Copyright (C) 2026 The Cacti Group. GPL version 2 or later.
|
||||
|
||||
test('link and plugin reorders reject GET and invalid tokens before writing (#7454)', function () {
|
||||
$root = dirname(__DIR__, 4);
|
||||
$parser = (new PhpParser\ParserFactory())->createForNewestSupportedVersion();
|
||||
$finder = new PhpParser\NodeFinder();
|
||||
$printer = new PhpParser\PrettyPrinter\Standard();
|
||||
$helper = $finder->findFirst($parser->parse(file_get_contents($root . '/include/csrf.php')),
|
||||
static fn ($node) => $node instanceof PhpParser\Node\Stmt\Function_ && $node->name->toString() === 'csrf_require_post');
|
||||
expect($helper)->not->toBeNull();
|
||||
$guard = $printer->prettyPrint([$helper]);
|
||||
|
||||
foreach (['links.php', 'plugins.php'] as $file) {
|
||||
$nodes = $parser->parse(file_get_contents($root . '/' . $file));
|
||||
$case = $finder->findFirst($nodes, static fn ($node) => $node instanceof PhpParser\Node\Stmt\Case_
|
||||
&& $node->cond instanceof PhpParser\Node\Scalar\String_ && $node->cond->value === 'ajax_dnd');
|
||||
expect($case)->not->toBeNull();
|
||||
$body = $printer->prettyPrint($case->stmts);
|
||||
|
||||
foreach ([['GET', null, 405], ['GET', 'valid', 405], ['HEAD', 'valid', 405], ['PUT', 'valid', 405], [null, 'valid', 405], ['POST', null, 403], ['POST', 'invalid', 403], ['POST', 'valid', 302]] as $scenario) {
|
||||
[$method, $token, $status] = $scenario;
|
||||
$seed = var_export(['method' => $method, 'token' => $token], true);
|
||||
$script = '<?php $seed = ' . $seed . ';' . <<<'PHP'
|
||||
|
||||
$_SERVER['REQUEST_METHOD'] = $seed['method'];
|
||||
http_response_code(200);
|
||||
$writes = 0;
|
||||
register_shutdown_function(function () { echo json_encode(array('status' => http_response_code(), 'writes' => $GLOBALS['writes'])); });
|
||||
function csrf_check($fatal) { return $GLOBALS['seed']['token'] === 'valid'; }
|
||||
function gnrv($name) { return array(2, 1); }
|
||||
function links_reorder($order) { $GLOBALS['writes']++; }
|
||||
function api_plugin_reorder($order) { $GLOBALS['writes']++; }
|
||||
PHP;
|
||||
$script .= "\n" . $guard . "\nswitch ('ajax_dnd') { case 'ajax_dnd':\n" . $body . "\n}";
|
||||
$process = proc_open([PHP_BINARY], [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes);
|
||||
fwrite($pipes[0], $script);
|
||||
fclose($pipes[0]);
|
||||
$result = json_decode(stream_get_contents($pipes[1]), true);
|
||||
$stderr = stream_get_contents($pipes[2]);
|
||||
fclose($pipes[1]);
|
||||
fclose($pipes[2]);
|
||||
expect(proc_close($process))->toBe(0);
|
||||
expect($stderr)->toBe('');
|
||||
expect($result)->toBe(['status' => $status, 'writes' => $status === 302 ? 1 : 0]);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('reorder clients submit their action and serialized order through the CSRF POST helper', function () {
|
||||
$root = dirname(__DIR__, 4);
|
||||
|
||||
foreach (['links.php', 'plugins.php'] as $file) {
|
||||
$source = file_get_contents($root . '/' . $file);
|
||||
expect($source)->toContain("cactiPreparePostRequest('" . $file . "', 'action=ajax_dnd&' + $.tableDnD.serialize())");
|
||||
expect($source)->toContain('postUrl({url: request.url}, request.data)');
|
||||
expect($source)->not->toContain("loadUrl({url:'" . $file . '?action=ajax_dnd');
|
||||
}
|
||||
});
|
||||
@@ -199,5 +199,3 @@ test('a dangling symlink below the RRA directory is rejected even for a not-yet-
|
||||
unlink($link);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -47,6 +47,25 @@ const context = {
|
||||
|
||||
vm.runInNewContext(`${extractFunction(source, 'cactiPreparePostRequest')}\n${extractFunction(source, 'cactiPreparePostRequestFromUrl')}`, context);
|
||||
|
||||
for (const page of ['links.php', 'plugins.php']) {
|
||||
test(`${page} drag-and-drop sends the serialized order and trusted CSRF token in a POST body`, () => {
|
||||
const pageSource = fs.readFileSync(path.join(__dirname, '..', '..', page), 'utf8');
|
||||
const callback = pageSource.match(/const request = cactiPreparePostRequest\([^\n]+\);\s*postUrl\(\{url: request\.url\}, request\.data\);/);
|
||||
assert.ok(callback, 'drag-and-drop must use the POST helper');
|
||||
let sent;
|
||||
const callbackContext = {...context,
|
||||
$: {...context.$, tableDnD: {serialize: () => 'dnd%5B%5D=2&dnd%5B%5D=1'}},
|
||||
postUrl: (options, data) => { sent = {options, data}; },
|
||||
};
|
||||
vm.runInNewContext(callback[0], callbackContext);
|
||||
assert.equal(sent.options.url, `/cacti/${page}`);
|
||||
const fields = new URLSearchParams(sent.data);
|
||||
assert.equal(fields.get('action'), 'ajax_dnd');
|
||||
assert.deepEqual(fields.getAll('dnd[]'), ['2', '1']);
|
||||
assert.equal(fields.get('__csrf_magic'), 'trusted-token');
|
||||
});
|
||||
}
|
||||
|
||||
test('state-changing URL fields move into a same-origin POST body with the trusted token', () => {
|
||||
const request = context.cactiPreparePostRequestFromUrl('/cacti/cdef.php?action=item_remove&id=7&__csrf_magic=attacker');
|
||||
const fields = new URLSearchParams(request.data);
|
||||
|
||||
Reference in new issue
Block a user