ddns-scripts: fix root command injection through ddns_dateformat

The date format read from the UCI option ddns.global.ddns_dateformat was
embedded into a command string that is later executed through "eval", both
for the recurring log timestamps

  DATE_PROG="date +'$ddns_dateformat'"
  ...
  write_log 5 "PID '$$' started at $(eval $DATE_PROG)"

and for the "last update" timestamp in the updater

  EPOCH_TIME="date -d @$EPOCH_TIME +'$ddns_dateformat'"
  write_log 7 "last update: $(eval $EPOCH_TIME)"

The value is only wrapped in single quotes, so a single quote inside it
closes that quote and starts a new shell word. A user who can write the ddns
configuration - a delegated DDNS operator who was never granted shell access
- can set

  ddns_dateformat="%F'; touch /tmp/pwned; '"

and have arbitrary commands run as root, because the updater runs as root.
The injection triggers on the next start of the updater, so in practice on
the next reconfiguration or reboot.

Stop building shell code from the option. Provide date_prog() as an ordinary
shell function and pass the format as a single quoted argument in both
places, which leaves no way for its content to be interpreted by the shell.
The remaining use of ddns_dateformat in dynamic_dns_updater.sh, for
NEXT_CHECK_TIME, already substituted it as a quoted argument without eval
and was not affected.

luci-app-ddns consumes the same option and is fixed separately in the LuCI
repository.

Reported-by: Matthew Hickey (Hacker Fantastic, https://hacker.house)
Fixes: 1c20dcb71a69 ("ddns-scripts: update to 2.7.6-1")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Hauke Mehrtens <redacted>

diff --git a/net/ddns-scripts/Makefile b/net/ddns-scripts/Makefile
index 0f60f2182a86dd946b33ee32e39c78fa95bc79e5..e3152da64db33b6808029bdf2d317e44a6305cd5 100644 (file)
--- a/net/ddns-scripts/Makefile
+++ b/net/ddns-scripts/Makefile
@@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=ddns-scripts
 PKG_VERSION:=2.8.4
-PKG_RELEASE:=1
+PKG_RELEASE:=2
 
 PKG_LICENSE:=GPL-2.0
 

diff --git a/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_functions.sh b/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_functions.sh
index 0f4c5d67160d35761f9ca4c607cebf6ed73614f0..a8ba663ec2e10c40da1d34d7bc20b4a8051f734d 100644 (file)
--- a/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_functions.sh
+++ b/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_functions.sh
@@ -121,7 +121,11 @@ ddns_loglines=$((ddns_loglines + 1))       # correct sed handling
 
 # format to show date information in log and luci-app-ddns default ISO 8601 format
 ddns_dateformat=$(uci -q get ddns.global.ddns_dateformat) || ddns_dateformat="%F %R"
-DATE_PROG="date +'$ddns_dateformat'"
+
+# Print the current date using the configured format.
+# Do not turn this into a command string that is run through "eval": the format
+# is read from UCI and a single quote inside it would start a new shell word.
+date_prog() { date +"$ddns_dateformat"; }
 
 # USE_CURL if GNU Wget and cURL installed normally Wget is used by do_transfer()
 # to change this use global option use_curl '1'
@@ -1121,17 +1125,17 @@ trap_handler() {
 
        case $1 in
                 0)     if [ $__ERR -eq 0 ]; then
-                               write_log 5 "PID '$$' exit normal at $(eval $DATE_PROG)${N}"
+                               write_log 5 "PID '$$' exit normal at $(date_prog)${N}"
                        else
-                               write_log 4 "PID '$$' exit WITH ERROR '$__ERR' at $(eval $DATE_PROG)${N}"
+                               write_log 4 "PID '$$' exit WITH ERROR '$__ERR' at $(date_prog)${N}"
                        fi ;;
-                1)     write_log 6 "PID '$$' received 'SIGHUP' at $(eval $DATE_PROG)"
+                1)     write_log 6 "PID '$$' received 'SIGHUP' at $(date_prog)"
                        # reload config via starting the script again
                        /usr/lib/ddns/dynamic_dns_updater.sh -v "0" -S "$__SECTIONID" -- start || true
                        exit 0 ;;       # and leave this one
-                2)     write_log 5 "PID '$$' terminated by 'SIGINT' at $(eval $DATE_PROG)${N}";;
-                3)     write_log 5 "PID '$$' terminated by 'SIGQUIT' at $(eval $DATE_PROG)${N}";;
-               15)     write_log 5 "PID '$$' terminated by 'SIGTERM' at $(eval $DATE_PROG)${N}";;
+                2)     write_log 5 "PID '$$' terminated by 'SIGINT' at $(date_prog)${N}";;
+                3)     write_log 5 "PID '$$' terminated by 'SIGQUIT' at $(date_prog)${N}";;
+               15)     write_log 5 "PID '$$' terminated by 'SIGTERM' at $(date_prog)${N}";;
                 *)     write_log 13 "Unhandled signal '$1' in 'trap_handler()'";;
        esac
 

diff --git a/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_updater.sh b/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_updater.sh
index b01697c6d0e4db605b8292c0963f166382859ff8..0d61feca1d9a8e0d9695eb1050d653e3f8ab820d 100644 (file)
--- a/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_updater.sh
+++ b/net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_updater.sh
@@ -174,14 +174,14 @@ ERR_LAST=$?       # save return code - equal 0 if SECTION_ID found
        [ $VERBOSE -le 1 ] && VERBOSE=2         # force console out and logfile output
        [ -f $LOGFILE ] && rm -f $LOGFILE       # clear logfile before first entry
        write_log  7 "************ ************** ************** **************"
-       write_log  5 "PID '$$' started at $(eval $DATE_PROG)"
+       write_log  5 "PID '$$' started at $(date_prog)"
        write_log  7 "ddns version  : $VERSION"
        write_log  7 "uci configuration:${N}$(uci -q show ddns | grep '=service' | sort)"
        write_log 14 "Service section '$SECTION_ID' not defined"
 }
 
 write_log 7 "************ ************** ************** **************"
-write_log 5 "PID '$$' started at $(eval $DATE_PROG)"
+write_log 5 "PID '$$' started at $(date_prog)"
 write_log 7 "ddns version  : $VERSION"
 write_log 7 "uci configuration:${N}$(uci -q show ddns.$SECTION_ID | sort)"
 # write_log 7 "ddns version  : $(opkg list-installed ddns-scripts | cut -d ' ' -f 3)"
@@ -285,8 +285,7 @@ if [ $LAST_TIME -eq 0 ]; then
        write_log 7 "last update: never"
 else
        EPOCH_TIME=$(( $(date +%s) - $CURR_TIME + $LAST_TIME ))
-       EPOCH_TIME="date -d @$EPOCH_TIME +'$ddns_dateformat'"
-       write_log 7 "last update: $(eval $EPOCH_TIME)"
+       write_log 7 "last update: $(date -d @$EPOCH_TIME +"$ddns_dateformat")"
 fi
 
 # verify Proxy server and set environment
@@ -309,7 +308,7 @@ ERR_LAST=$?
 [ $use_ipv6 -eq 1 ] && expand_ipv6 "$REGISTERED_IP" REGISTERED_IP
 
 # loop endlessly, checking ip every check_interval and forcing an updating once every force_interval
-write_log 6 "Starting main loop at $(eval $DATE_PROG)"
+write_log 6 "Starting main loop at $(date_prog)"
 while : ; do
 
        get_current_ip CURRENT_IP               # read current IP
@@ -396,7 +395,7 @@ while : ; do
        [ $FORCE_SECONDS -eq 0 ] && write_log 6 "Configured to run once"
        [ $VERBOSE -gt 1 -o $FORCE_SECONDS -eq 0 ] && exit 0
 
-       write_log 6 "Rerun IP check at $(eval $DATE_PROG)"
+       write_log 6 "Rerun IP check at $(date_prog)"
 done
 # we should never come here there must be a programming error
 write_log 12 "Error in 'dynamic_dns_updater.sh - program coding error"