Skip to content

hardening: migrate flowview process/version lookups to prepared #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
somethingwithproof wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

hardening: migrate flowview process/version lookups to prepared #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85312dc
security: migrate flowview process/version lookups to prepared
somethingwithproof Mar 15, 2026
3a1079c
fix: reduce brittleness in prepared regression checks
somethingwithproof Mar 16, 2026
da408cb
test: add grok follow-up parameter binding checks for flowview
somethingwithproof Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions flowview_devices.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,10 @@ function save_device() {

$id = flowview_sql_save($save, 'plugin_flowview_devices', 'id', true);

$pid = db_fetch_cell('SELECT pid FROM processes WHERE tasktype="flowview" AND taskname="master"');
$pid = db_fetch_cell_prepared('SELECT pid
FROM processes
WHERE tasktype = ?
AND taskname = ?', array('flowview', 'master'));

if (is_error_message()) {
raise_message(2);
Expand All @@ -320,10 +323,10 @@ function save_device() {
}

function restart_services() {
$pid = db_fetch_cell('SELECT pid
$pid = db_fetch_cell_prepared('SELECT pid
FROM processes
WHERE tasktype="flowview"
AND taskname="master"');
WHERE tasktype = ?
AND taskname = ?', array('flowview', 'master'));

if ($pid > 0) {
if (!defined('SIGHUP')) {
Expand Down Expand Up @@ -974,4 +977,3 @@ function clearFilter() {

form_end();
}

10 changes: 6 additions & 4 deletions setup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ function plugin_flowview_check_upgrade($force = false) {
$info = plugin_flowview_version();
$current = $info['version'];

$old = db_fetch_cell('SELECT version
$old = db_fetch_cell_prepared('SELECT version
FROM plugin_config
WHERE directory="flowview"');
WHERE directory = ?', array('flowview'));

if ($current != $old || $force) {
$php_binary = read_config_option('path_php_binary');
Expand Down Expand Up @@ -321,7 +321,10 @@ function flowview_global_settings_update() {
}

if ($hup_process) {
$pid = db_fetch_cell('SELECT pid FROM processes WHERE tasktype="flowview" AND taskname="master"');
$pid = db_fetch_cell_prepared('SELECT pid
FROM processes
WHERE tasktype = ?
AND taskname = ?', array('flowview', 'master'));

if ($pid > 0) {
if (!defined('SIGHUP')) {
Expand Down Expand Up @@ -1309,4 +1312,3 @@ function flowview_graph_button($data) {
}
}
}

89 changes: 89 additions & 0 deletions tests/test_prepared_statements.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

function assert_contains($haystack, $needle, $message) {
if (strpos($haystack, $needle) === false) {
fwrite(STDERR, $message . PHP_EOL);
exit(1);
}
}

function assert_not_contains($haystack, $needle, $message) {
if (strpos($haystack, $needle) !== false) {
fwrite(STDERR, $message . PHP_EOL);
exit(1);
}
}

function assert_regex($pattern, $subject, $message) {
if (!preg_match($pattern, $subject)) {
fwrite(STDERR, $message . PHP_EOL);
exit(1);
}
}

function assert_not_regex($pattern, $subject, $message) {
if (preg_match($pattern, $subject)) {
fwrite(STDERR, $message . PHP_EOL);
exit(1);
}
}

$devices = file_get_contents(__DIR__ . '/../flowview_devices.php');
if ($devices === false) {
fwrite(STDERR, "Unable to read flowview_devices.php\n");
exit(1);
}

assert_contains(
$devices,
"db_fetch_cell_prepared('SELECT pid",
'Expected flowview_devices.php process lookup to use db_fetch_cell_prepared().'
);
assert_regex(
"/db_fetch_cell_prepared\\s*\\(\\s*'SELECT\\s+pid[\\s\\S]*tasktype\\s*=\\s*\\?[\\s\\S]*taskname\\s*=\\s*\\?[\\s\\S]*array\\(\\s*'flowview'\\s*,\\s*'master'\\s*\\)\\s*\\)/s",
$devices,
'Expected flowview_devices.php process lookup to bind flowview/master via placeholders.'
);

assert_not_regex(
"/db_fetch_cell\\s*\\(\\s*['\\\"]SELECT\\s+pid\\s+FROM\\s+processes\\s+WHERE\\s+tasktype\\s*=\\s*['\\\"]flowview['\\\"]\\s+AND\\s+taskname\\s*=\\s*['\\\"]master['\\\"]/is",
$devices,
'Raw process lookup should not remain in flowview_devices.php.'
);

$setup = file_get_contents(__DIR__ . '/../setup.php');
if ($setup === false) {
fwrite(STDERR, "Unable to read setup.php\n");
exit(1);
}

assert_contains(
$setup,
"db_fetch_cell_prepared('SELECT version",
'Expected setup.php plugin version lookup to use db_fetch_cell_prepared().'
);
assert_regex(
"/db_fetch_cell_prepared\\s*\\(\\s*'SELECT\\s+pid[\\s\\S]*tasktype\\s*=\\s*\\?[\\s\\S]*taskname\\s*=\\s*\\?[\\s\\S]*array\\(\\s*'flowview'\\s*,\\s*'master'\\s*\\)\\s*\\)/s",
$setup,
'Expected setup.php process lookup to bind flowview/master via placeholders.'
);

assert_regex(
"/WHERE\\s+directory\\s*=\\s*\\?\\s*'?,\\s*array\\(\\s*'flowview'\\s*\\)\\s*\\)/s",
$setup,
'Expected setup.php version lookup to bind flowview directory via placeholder.'
);

assert_not_contains(
$setup,
'db_fetch_cell(\'SELECT version',
'Raw version lookup should not remain in setup.php.'
);

assert_not_regex(
"/db_fetch_cell\\s*\\(\\s*['\\\"]SELECT\\s+pid\\s+FROM\\s+processes\\s+WHERE\\s+tasktype\\s*=\\s*['\\\"]flowview['\\\"]\\s+AND\\s+taskname\\s*=\\s*['\\\"]master['\\\"]/is",
$setup,
'Raw process lookup should not remain in setup.php.'
);

echo "OK\n";