Skip to content

Commit 043d86c

Browse files
SECURITY FIX - Only allow system page and script modules to run from ~/allsky/config/myFiles
1 parent 17dfe2c commit 043d86c

6 files changed

Lines changed: 431 additions & 112 deletions

File tree

html/includes/moduleutil.php

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -675,15 +675,80 @@ private function addSecretsToFlow($configData) {
675675
return $configData;
676676
}
677677

678-
public function postTestModule(): void
679-
{
678+
private function sendModuleTestValidationResult(string $message): void
679+
{
680+
$payload = [
681+
'message' => $message,
682+
'extradata' => (object)[],
683+
];
684+
685+
$this->sendResponse(json_encode($payload, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT));
686+
}
687+
688+
private function validateRunScriptModuleTest(array $flowData): ?string
689+
{
690+
$myFilesDir = realpath((string)ALLSKY_MYFILES_DIR);
691+
if ($myFilesDir === false || !is_dir($myFilesDir)) {
692+
return "Allsky could not test this module because the configured scripts folder is not available.\n\nExpected scripts folder: " . (string)ALLSKY_MYFILES_DIR . "\n\nCheck the scripts folder setting in variables.json and make sure that folder exists.";
693+
}
694+
695+
foreach ($flowData as $moduleData) {
696+
if (!is_array($moduleData) || ($moduleData['module'] ?? '') !== 'allsky_script.py') {
697+
continue;
698+
}
699+
700+
$script = trim((string)($moduleData['metadata']['arguments']['scriptlocation'] ?? ''));
701+
if ($script === '') {
702+
return "Allsky could not test this module because no script has been selected.\n\nChoose a script from " . $myFilesDir . " and try the test again.";
703+
}
704+
705+
if ($script[0] !== '/') {
706+
return "Allsky could not test this module because the selected script is not a full file path.\n\nSelected script: " . $script . "\n\nChoose the script again from " . $myFilesDir . " so Allsky can save the full path.";
707+
}
708+
709+
$realScript = realpath($script);
710+
if ($realScript === false || !is_file($realScript)) {
711+
return "Allsky could not test this module because it cannot find the selected script.\n\nSelected script: " . $script . "\n\nThe script may have been deleted, renamed, or moved. Put the script in " . $myFilesDir . " and select it again.";
712+
}
713+
714+
if (!$this->isPathWithinDirectory($realScript, $myFilesDir)) {
715+
return "Allsky could not test this module because the selected script is outside the allowed scripts folder.\n\nSelected script: " . $realScript . "\nAllowed scripts folder: " . $myFilesDir . "\n\nMove the script into the allowed folder, select it again in the module settings, and then run the test again.";
716+
}
717+
718+
if (!is_readable($realScript)) {
719+
return "Allsky could not test this module because it cannot read the selected script.\n\nSelected script: " . $realScript . "\n\nCheck the file permissions for the script. Allsky needs permission to read the script before it can run it.";
720+
}
721+
722+
if (!is_executable($realScript)) {
723+
return "Allsky could not test this module because the selected script is not marked as executable.\n\nSelected script: " . $realScript . "\n\nMark the script as executable, then run the test again. If you are not sure how to do this, ask whoever supplied the script to check its permissions.";
724+
}
725+
}
726+
727+
return null;
728+
}
729+
730+
private function isPathWithinDirectory(string $path, string $directory): bool
731+
{
732+
$directory = rtrim($directory, '/');
733+
return $path === $directory || strpos($path, $directory . '/') === 0;
734+
}
735+
736+
public function postTestModule(): void
737+
{
680738
// Read and sanitize inputs
681739
$module = trim((string)filter_input(INPUT_POST, 'module', FILTER_UNSAFE_RAW));
682740
$dayNight = trim((string)filter_input(INPUT_POST, 'dayNight', FILTER_UNSAFE_RAW));
683741
$flow = (string)($_POST['flow'] ?? '');
684742

685743
// Inject any required secrets into the flow
686744
$flow = $this->addSecretsToFlow($flow);
745+
$jsonFlow = json_decode($flow, true);
746+
if (is_array($jsonFlow)) {
747+
$validationMessage = $this->validateRunScriptModuleTest($jsonFlow);
748+
if ($validationMessage !== null) {
749+
$this->sendModuleTestValidationResult($validationMessage);
750+
}
751+
}
687752

688753
// Save the flow definition to a temp JSON file
689754
$fileName = ALLSKY_MODULES . '/test_flow.json';
@@ -704,7 +769,6 @@ public function postTestModule(): void
704769
$result = $this->runProcess($argv);
705770

706771
// Try to load extradata if the flow references a file
707-
$jsonFlow = json_decode($flow, true);
708772
$extraData = '';
709773
$moduleKey = is_array($jsonFlow) ? array_key_first($jsonFlow) : null;
710774

0 commit comments

Comments
 (0)