Files
cacti/tests/Unit/Security/Shell/RrdQuerySubstitutionEscapeTest.php
Thomas Vincent d07758c48b security: substitute query data before escaping graph titles and axis labels (#7846)
* security: substitute query data before escaping graph titles and labels

rrd_substitute_host_query_data() ran over the whole graph command after each
argument was cacti_escapeshellarg'd, so a device-supplied SNMP value expanded
from a |query_*| token in a graph title, vertical-label or right-axis-label
could contain a quote and break out of the RRDtool argument. Move the
substitution inside cacti_escapeshellarg at those three fields and drop the
post-escape whole-command pass. Forward-port of the 1.2.x fix.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* security: finish resolve-before-escape refactor, drop redundant inner substitution

$value is already resolved (credential-stripped and host/query substituted) by
rrdtool_resolve_graph_text() at the top of the graph-options loop, so the inner
rrd_substitute_host_query_data() in the title, vertical-label and right-axis-label
argument builders was a redundant second pass. Reconcile the older escape test to
match; the device-quote-neutralisation behavior test is unchanged.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-02 20:32:47 -07:00

40 lines
1.8 KiB
PHP

<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
/*
* Regression: |query_*|/|host_*| tokens in a graph title, vertical-label or
* right-axis-label are replaced with device-supplied SNMP values. The value is
* resolved (credential-stripped and substituted) by rrdtool_resolve_graph_text()
* before it reaches cacti_escapeshellarg, so a device value that contains a quote
* is escaped and cannot break out of the RRDtool argument; the post-escape
* whole-command pass that previously injected into the quoted args must be gone.
*/
$rrd = file_get_contents(dirname(__DIR__, 4) . '/lib/rrd.php');
test('the resolved title value is escaped', function () use ($rrd) {
expect($rrd)->toContain('cacti_escapeshellarg(htmle($value))');
});
test('the resolved right-axis-label value is escaped', function () use ($rrd) {
expect($rrd)->toContain("--right-axis-label ' . cacti_escapeshellarg(\$value)");
});
test('the post-escape whole-command substitution is removed', function () use ($rrd) {
expect($rrd)->not->toContain('$graph_opts = rrd_substitute_host_query_data($graph_opts, $graph, []);');
});
test('cacti_escapeshellarg neutralises a device-supplied quote', function () {
require_once dirname(__DIR__, 4) . '/lib/functions.php';
$GLOBALS['config']['cacti_server_os'] = 'unix';
$wrapped = cacti_escapeshellarg("Traffic eth0' COMMENT:pwned");
expect($wrapped)->not->toContain("Traffic eth0' COMMENT");
expect(substr($wrapped, 0, 1))->toBe("'");
});