Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 13 additions & 2 deletions emhttp/languages/en_US/helptext.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2155,8 +2155,19 @@ Click on the counters to either acknowledge or view the unread notifications.
:end

:notifications_display_position_help:
Choose the position of where notifications appear on screen in *Detailed* view. Multiple notifications are stacked, bottom-to-top or
top-to-bottom depending on the selected placement.
Choose the position of where notification popups appear on screen.
:end

:notifications_stack_help:
When enabled, multiple notifications are stacked to conserve screen space.
:end

:notifications_duration_help:
Time in milliseconds before a notification automatically closes.
:end

:notifications_max_help:
Maximum number of notifications shown on screen at once.
:end

:notifications_auto_close_help:
Expand Down
37 changes: 28 additions & 9 deletions emhttp/plugins/dynamix/Notifications.page
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,44 @@ $(function(){
<input type="hidden" name="language_notify">
<input type="hidden" name="report">

_(Store notifications to flash)_:
: <select name="path">
<?=mk_option($notify['path'], "/tmp/notifications", _("No"))?>
<?=mk_option($notify['path'], "/boot/config/plugins/dynamix/notifications", _("Yes"))?>
</select>

:notifications_store_flash_help:

_(Display position)_:
: <select name="position">
<?=mk_option($notify['position'], "top-left", _("top-left"))?>
<?=mk_option($notify['position'], "top-right", _("top-right"))?>
<?=mk_option($notify['position'], "bottom-left", _("bottom-left"))?>
<?=mk_option($notify['position'], "bottom-right", _("bottom-right"))?>
<?=mk_option($notify['position'], "center", _("center"))?>
<?=mk_option($notify['position'], "bottom-center", _("bottom-center"))?>
<?=mk_option($notify['position'], "top-center", _("top-center"))?>
</select>

:notifications_display_position_help:

_(Stack notification popups)_:
: <select name="expand">
<?=mk_option($notify['expand'] ?? 'true', "true", _("Yes"))?>
<?=mk_option($notify['expand'] ?? 'true', "false", _("No"))?>
</select>

:notifications_stack_help:

_(Notification popup duration)_:
: <input type="number" name="duration" value="<?=$notify['duration'] ?? 5000?>" min="1000" step="500">

:notifications_duration_help:

_(Max notification popups)_:
: <input type="number" name="max" value="<?=$notify['max'] ?? 3?>" min="1" max="10">

:notifications_max_help:
Comment on lines +148 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the Notifications.page file and check for server-side validation

echo "=== Checking file structure ==="
# First, find the Notifications.page file
find . -name "Notifications.page" -type f

echo ""
echo "=== Checking lines 148-151 in Notifications.page ==="
# Read the specific lines mentioned in the review
sed -n '145,155p' emhttp/plugins/dynamix/Notifications.page

echo ""
echo "=== Searching for max popups validation in backend ==="
# Search for validation logic related to max popups
rg -i 'max.*notif|notify.*max' emhttp/plugins/dynamix/include/ -A 3 -B 1

echo ""
echo "=== Searching for validation patterns in notification handling ==="
# Look for any validation patterns in the notification files
rg '\$notify\[.*max|validate.*max|bounds.*max' emhttp/plugins/dynamix/ -A 2

echo ""
echo "=== Checking for POST/input validation in backend ==="
# Search for where the notification settings are processed/validated
rg -i 'post.*notif|input.*notif' emhttp/plugins/dynamix/include/ -l | head -10

Repository: unraid/webgui

Length of output: 50371


🏁 Script executed:

#!/bin/bash
# More targeted search for notification settings validation

echo "=== Looking for notification settings handler files ==="
find emhttp/plugins/dynamix/include/ -type f -name "*.php" -o -name "*.sh" | head -20

echo ""
echo "=== Searching for where notify array is processed ==="
rg '\$notify.*=.*\$_' emhttp/plugins/dynamix/include/ -A 2

echo ""
echo "=== Searching for notification settings save/update logic ==="
rg -l 'notify.*POST|POST.*notify' emhttp/plugins/dynamix/

echo ""
echo "=== Checking the main notification handler ==="
rg -l 'notifications' emhttp/plugins/dynamix/include/ | head -5

Repository: unraid/webgui

Length of output: 1762


🏁 Script executed:

#!/bin/bash
# Check the full Notifications.page file structure

echo "=== File size and line count ==="
wc -l emhttp/plugins/dynamix/Notifications.page

echo ""
echo "=== Checking for form submission handlers ==="
rg -i 'submit|POST|save.*notif' emhttp/plugins/dynamix/Notifications.page -B 2 -A 2

echo ""
echo "=== Checking for JavaScript validation ==="
rg -i 'validate.*max|max.*validate|function.*notif' emhttp/plugins/dynamix/Notifications.page

echo ""
echo "=== Checking entire file for validation logic ==="
# Read the full file to look for any server-side validation
cat emhttp/plugins/dynamix/Notifications.page | head -100

echo ""
echo "=== Search for where the form is saved/submitted ==="
rg 'apply|update' emhttp/plugins/dynamix/Notifications.page -i -B 1 -A 1

Repository: unraid/webgui

Length of output: 7884


🏁 Script executed:

#!/bin/bash
# Find and examine the update handler

echo "=== Looking for update.php ==="
find . -name "update.php" -type f | grep -v node_modules | grep -v ".js"

echo ""
echo "=== Checking update.php for validation logic ==="
if [ -f "emhttp/update.php" ]; then
  wc -l emhttp/update.php
  echo "---"
  # Look for validation patterns
  rg -i 'validate|sanitize|intval|bounds|range|min.*max' emhttp/update.php | head -20
fi

echo ""
echo "=== Searching for numeric field validation ==="
rg -i 'intval|is_numeric|\[0-9\]' emhttp/update.php | head -10

echo ""
echo "=== Check for specific max/min validation ==="
rg -C 3 '\$max|\bmax\b' emhttp/update.php | head -30

Repository: unraid/webgui

Length of output: 278


🏁 Script executed:

#!/bin/bash
# Read the full update.php file

echo "=== Full update.php content ==="
cat -n emhttp/update.php

echo ""
echo "=== Check if there's any validation in dynamix.cfg parsing ==="
rg -i 'notify|max' emhttp/plugins/dynamix/ --include="*.php" | grep -i "validate\|sanitize\|intval\|min\|max" | head -20

Repository: unraid/webgui

Length of output: 7555


🏁 Script executed:

#!/bin/bash
# Check for validation logic related to notifications

echo "=== Searching for notification-related includes or validation ==="
rg 'notify' emhttp/plugins/dynamix/ -t php | grep -i "valid\|sanitize\|intval\|bounds\|include" | head -20

echo ""
echo "=== Check if update.php uses an include for notify section ==="
grep -r "#include" emhttp/plugins/dynamix/Notifications.page

echo ""
echo "=== Look for dynamix.cfg default values ==="
find emhttp/plugins/dynamix -name "default.cfg" -o -name "dynamix.cfg" | xargs cat 2>/dev/null | head -40

echo ""
echo "=== Check for notify validation in wrapper functions ==="
rg 'function.*notify\|notify.*valid' emhttp/plugins/dynamix/include/ -t php

Repository: unraid/webgui

Length of output: 2791


🏁 Script executed:

#!/bin/bash
# Find and examine the notify script

echo "=== Looking for notify script ==="
find . -path "*/webGui/scripts/notify" -type f

echo ""
echo "=== Check for max validation in notify script ==="
if [ -f "emhttp/webGui/scripts/notify" ]; then
  file emhttp/webGui/scripts/notify
  echo "---"
  rg -i 'max.*popup|max.*notif|validate.*max' emhttp/webGui/scripts/notify | head -10
fi

echo ""
echo "=== Look for the full notify section in default.cfg ==="
find emhttp/plugins/dynamix -name "default.cfg" -exec grep -A 20 "\[notify\]" {} \;

echo ""
echo "=== Search for any bounds checking on numeric values ==="
rg -C 3 '\$notify\[.max.\]' emhttp/ -t php

Repository: unraid/webgui

Length of output: 615


Add server-side validation to enforce max popup bounds.

The max notification popups field has client-side HTML5 constraints (min=1, max=10) but update.php writes form values directly to the configuration file without any bounds checking. Add validation to reject values outside the 1-10 range before saving to prevent invalid configurations from direct POST requests or manual config editing.

🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/Notifications.page around lines 148-151, the "max"
input value is only constrained client-side; add server-side validation when
processing the form (e.g., in update.php or the handler invoked on POST) to
enforce an integer between 1 and 10. Specifically, retrieve the posted value,
cast to int, verify it is >=1 and <=10, and if not, reject the request (do not
write the invalid value to the config) and return a clear error message to the
user; if the value is missing or invalid type, treat it as an error as well.
Ensure you do not silently override invalid values—fail validation and prompt
correction before saving the configuration.


_(Date format)_:
: <select name="date">
<?=mk_option($notify['date'], "d-m-Y", _("DD-MM-YYYY"))?>
Expand All @@ -140,14 +167,6 @@ _(Time format)_:

:notifications_time_format_help:

_(Store notifications to flash)_:
: <select name="path">
<?=mk_option($notify['path'], "/tmp/notifications", _("No"))?>
<?=mk_option($notify['path'], "/boot/config/plugins/dynamix/notifications", _("Yes"))?>
</select>

:notifications_store_flash_help:

_(System notifications)_:
: <select name="system" onchange="prepareSystem(this.selectedIndex)">
<?=mk_option($notify['system'], "", _("Disabled"))?>
Expand Down
3 changes: 3 additions & 0 deletions emhttp/plugins/dynamix/default.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ day="0"
cron=""
write="NOCORRECT"
[notify]
expand="true"
duration="5000"
max="3"
date="d-m-Y"
time="H:i"
position="top-right"
Expand Down
4 changes: 4 additions & 0 deletions emhttp/plugins/dynamix/include/Notify.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
case 'm':
$notify .= " -{$option} ".escapeshellarg($value);
break;
case 'u':
$notify .= " -{$option} ".escapeshellarg($value);
break;

Comment on lines +39 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check the notify script for validation of the -u parameter and safe_filename function

# Find the notify script
NOTIFY_SCRIPT="emhttp/plugins/dynamix/scripts/notify"

if [ -f "$NOTIFY_SCRIPT" ]; then
  echo "=== Checking for -u option handling ==="
  rg -A 10 -B 2 'u\)' "$NOTIFY_SCRIPT" | head -50
  
  echo ""
  echo "=== Checking for safe_filename function ==="
  rg -A 20 'safe_filename|function.*filename' "$NOTIFY_SCRIPT"
  
  echo ""
  echo "=== Checking for filename validation patterns ==="
  rg -C 3 'basename|realpath|sanitize|validate.*filename' "$NOTIFY_SCRIPT"
else
  echo "Notify script not found at $NOTIFY_SCRIPT"
fi

Repository: unraid/webgui

Length of output: 3165


🏁 Script executed:

#!/bin/bash

NOTIFY_SCRIPT="emhttp/plugins/dynamix/scripts/notify"

# Search for case statement handling options
echo "=== Searching for all case statement handling ==="
rg -B 5 -A 10 "case '-" "$NOTIFY_SCRIPT" | head -80

echo ""
echo "=== Full context around customFilename usage ==="
rg -B 10 -A 10 "customFilename" "$NOTIFY_SCRIPT"

echo ""
echo "=== Check if .notify extension is enforced for custom filenames ==="
rg -B 5 -A 5 '\.notify' "$NOTIFY_SCRIPT"

Repository: unraid/webgui

Length of output: 2637


Extend -u parameter validation to enforce .notify extension.

The safe_filename() function in the notify script provides comprehensive protection against injection attacks and path traversal:

  • Removes dangerous special characters and enforces alphanumeric, dots, dashes, and underscores
  • Prevents path traversal by removing / and \ characters
  • Enforces reasonable length limits (maxLength parameter)

However, there is a validation gap: the custom filename passed via the -u option is not required to end with .notify. The default notification path enforces this via suffix construction (_{$ticket}.notify), but custom filenames bypass this requirement. Since the notification retrieval logic expects *.notify files (glob("$unread/*.notify", GLOB_NOSORT)), custom filenames without the .notify extension will not be retrievable by the notification system.

Add extension validation for custom filenames to either append or enforce the .notify extension.

🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/include/Notify.php around lines 39 to 42, the -u
option accepts a custom filename but does not enforce the .notify extension so
such files won't be picked up by the notification glob; after sanitizing the
provided filename with safe_filename(), ensure it ends with ".notify"
(case-insensitive) and if not, append ".notify" to the sanitized name, then use
that resulting filename when building $notify (i.e. escapeshellarg the
validated/normalized filename) so custom names are always retrievable by the
notification system.

case 'x':
case 't':
$notify .= " -{$option}";
Expand Down
8 changes: 4 additions & 4 deletions emhttp/plugins/dynamix/include/Translations.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ function parse_plugin($plugin) {
global $docroot,$language,$locale;
$plugin = strtolower($plugin);
$text = "$docroot/languages/$locale/$plugin.txt";
if (file_exists($text)) {
if (file_exists($text)) {
$store = "$docroot/languages/$locale/$plugin.dot";
if (!file_exists($store)) file_put_contents($store,serialize(parse_lang_file($text)));
clearstatcache(); if (!file_exists($store) || filemtime($text) > filemtime($store)) file_put_contents($store,serialize(parse_lang_file($text)));
$language = array_merge($language,unserialize(file_get_contents($store)));
}
}
Expand Down Expand Up @@ -133,7 +133,7 @@ function translate($key) {
if (file_exists($text)) {
$store = "$docroot/languages/$locale/translations.dot";
// global translations
if (!file_exists($store)) file_put_contents($store,serialize(parse_lang_file($text)));
clearstatcache(); if (!file_exists($store) || filemtime($text) > filemtime($store)) file_put_contents($store,serialize(parse_lang_file($text)));
$language = unserialize(file_get_contents($store));
}
if (file_exists("$docroot/languages/$locale/helptext.txt")) {
Expand Down Expand Up @@ -163,7 +163,7 @@ function translate($key) {
if (file_exists($text)) {
// additional translations
$store = "$docroot/languages/$locale/$more.dot";
if (!file_exists($store)) file_put_contents($store,serialize(parse_lang_file($text)));
clearstatcache(); if (!file_exists($store) || filemtime($text) > filemtime($store)) file_put_contents($store,serialize(parse_lang_file($text)));
$language = array_merge($language,unserialize(file_get_contents($store)));
}
}
Expand Down
37 changes: 28 additions & 9 deletions emhttp/plugins/dynamix/scripts/notify
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ notify [-e "event"] [-s "subject"] [-d "description"] [-i "normal|warning|alert"
use -r to specify recipients and not use default
use -t to force send email only (for testing)
use -b to NOT send a browser notification
use -u to specify a custom filename (API use only)
all options are optional

notify init
Expand Down Expand Up @@ -87,12 +88,13 @@ function generate_email($event, $subject, $description, $importance, $message, $
return mail($to, $subj, implode("\n", $body), implode("\n", $headers));
}

function safe_filename($string) {
function safe_filename($string, $maxLength=255) {
$special_chars = ["?", "[", "]", "/", "\\", "=", "<", ">", ":", ";", ",", "'", "\"", "&", "$", "#", "*", "(", ")", "|", "~", "`", "!", "{", "}"];
$string = trim(str_replace($special_chars, "", $string));
$string = preg_replace('~[^0-9a-z -_]~i', '', $string);
$string = preg_replace('~[^0-9a-z -_.]~i', '', $string);
$string = preg_replace('~[- ]~i', '_', $string);
return trim($string);
// limit filename length to $maxLength characters
return substr(trim($string), 0, $maxLength);
}

/*
Expand All @@ -112,7 +114,7 @@ function ini_encode_value($value) {
if (is_int($value) || is_float($value)) return $value;
if (is_bool($value)) return $value ? 'true' : 'false';
$value = (string)$value;
return '"'.strtr($value, ["\\"=>"\\\\", '"' => '\\"']).'"';
return '"'.strtr($value, ["\\" => "\\\\", '"' => '\\"']).'"';
}

function build_ini_string(array $data) {
Expand All @@ -135,6 +137,7 @@ function ini_decode_value($value) {
return $value;
}


// start
if ($argc == 1) exit(usage());

Expand Down Expand Up @@ -209,8 +212,9 @@ case 'add':
$mailtest = false;
$overrule = false;
$noBrowser = false;
$customFilename = false;

$options = getopt("l:e:s:d:i:m:r:xtb");
$options = getopt("l:e:s:d:i:m:r:u:xtb");
foreach ($options as $option => $value) {
switch ($option) {
case 'e':
Expand Down Expand Up @@ -246,11 +250,26 @@ case 'add':
$link = $value;
$fqdnlink = (strpos($link,"http") === 0) ? $link : ($nginx['NGINX_DEFAULTURL']??'').$link;
break;
case 'u':
$customFilename = $value;
break;
}
}

$unread = "{$unread}/".safe_filename("{$event}-{$ticket}.notify");
$archive = "{$archive}/".safe_filename("{$event}-{$ticket}.notify");
if ($customFilename) {
$filename = safe_filename($customFilename);
} else {
// suffix length: _{timestamp}.notify = 1+10+7 = 18 chars.
$suffix = "_{$ticket}.notify";
$max_name_len = 255 - strlen($suffix);
// sanitize event, truncating it to leave room for suffix
$clean_name = safe_filename($event, $max_name_len);
// construct filename with suffix (underscore separator matches safe_filename behavior)
$filename = "{$clean_name}{$suffix}";
}

$unread = "{$unread}/{$filename}";
$archive = "{$archive}/{$filename}";
if (file_exists($archive)) break;
Comment on lines +259 to 273
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Custom filenames lack the .notify extension required for notification discovery.

When a custom filename is provided (line 260), it's sanitized without enforcing the .notify extension. However, the get operation uses glob("$unread/*.notify", GLOB_NOSORT) to discover notifications. Custom filenames without this extension won't be found by the notification retrieval system, creating an inconsistency between custom and auto-generated notification filenames.

The auto-generated path (lines 264-268) explicitly includes the .notify extension as a suffix, while the custom filename path does not. This means a notification created with a custom filename would be stored but unretrievable via the get operation.

Recommend either:

  1. Appending .notify to custom filenames, or
  2. Documenting that custom filenames must explicitly include the .notify extension

Regarding the file_exists($archive) check (line 273): This appears to be an intentional loop exit and is not an issue.

🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/scripts/notify around lines 259-273, the branch that
handles $customFilename currently sanitizes the name but does not ensure it ends
with the required ".notify" extension, so custom-named notifications won't be
discovered by glob("$unread/*.notify"). After calling
safe_filename($customFilename) ensure the returned name ends with ".notify"
(append ".notify" if missing) before assigning to $filename; alternatively
sanitize the base name and then append the ".notify" suffix to guarantee
consistent discovery by the existing get logic.

$entity = $overrule===false ? $notify[$importance] : $overrule;
$cleanSubject = clean_subject($subject);
Expand All @@ -274,8 +293,8 @@ case 'add':
];
file_put_contents($unread, build_ini_string($unreadData));
}
if (($entity & 2)==2 || $mailtest) generate_email($event, $cleanSubject, str_replace('<br>','. ',$description), $importance, $message, $recipients, $fqdnlink);
if (($entity & 4)==4 && !$mailtest) { if (is_array($agents)) {foreach ($agents as $agent) {exec("TIMESTAMP='$timestamp' EVENT=".escapeshellarg($event)." SUBJECT=".escapeshellarg($cleanSubject)." DESCRIPTION=".escapeshellarg($description)." IMPORTANCE=".escapeshellarg($importance)." CONTENT=".escapeshellarg($message)." LINK=".escapeshellarg($fqdnlink)." bash ".$agent);};}};
if (($entity & 2)==2 || $mailtest) generate_email($event, clean_subject($subject), str_replace('<br>','. ',$description), $importance, $message, $recipients, $fqdnlink);
if (($entity & 4)==4 && !$mailtest) { if (is_array($agents)) {foreach ($agents as $agent) {exec("TIMESTAMP='$timestamp' EVENT=".escapeshellarg($event)." SUBJECT=".escapeshellarg(clean_subject($subject))." DESCRIPTION=".escapeshellarg($description)." IMPORTANCE=".escapeshellarg($importance)." CONTENT=".escapeshellarg($message)." LINK=".escapeshellarg($fqdnlink)." bash ".$agent);};}};
break;

case 'get':
Expand Down
Loading