From 9e79e1b668e010584b28cd771d0b97d5e5c50e99 Mon Sep 17 00:00:00 2001 From: Karl Vogel Date: Mon, 23 Jul 2018 10:52:42 +0200 Subject: [PATCH] ddns-scripts: sanitize host charset and shell escape characters Since certain characters are dangerous to pass as-is to a sub shell, sanitize the character set and only allow characters that are considered valid for DNS hosts and filter shell escape characters on generic parameters. Disable pathname expansion on RUNPROG evals to disable the shell expanding *, ? and [ in the arguments. Signed-off-by: Karl Vogel --- .../files/dynamic_dns_functions.sh | 61 ++++++++++++++++--- net/ddns-scripts/files/dynamic_dns_updater.sh | 9 +++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/net/ddns-scripts/files/dynamic_dns_functions.sh b/net/ddns-scripts/files/dynamic_dns_functions.sh index 845cc2b003..4ad9e60ed6 100755 --- a/net/ddns-scripts/files/dynamic_dns_functions.sh +++ b/net/ddns-scripts/files/dynamic_dns_functions.sh @@ -63,6 +63,12 @@ IPV4_REGEX="[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}" # IPv6 ( ( 0-9a-f 1-4char ":") min 1x) ( ( 0-9a-f 1-4char )optional) ( (":" 0-9a-f 1-4char ) min 1x) IPV6_REGEX="\(\([0-9A-Fa-f]\{1,4\}:\)\{1,\}\)\(\([0-9A-Fa-f]\{1,4\}\)\{0,1\}\)\(\(:[0-9A-Fa-f]\{1,4\}\)\{1,\}\)" +# characters that are dangerous to pass to a shell command line +SHELL_ESCAPE="[\"\'\`\$\!();><{}?|\[\]\*\\\\]" + +# dns character set +DNS_CHARSET="[@a-zA-Z0-9._-]" + # detect if called by ddns-lucihelper.sh script, disable retrys (empty variable == false) LUCI_HELPER=$(printf %s "$MYPROG" | grep -i "luci") @@ -467,6 +473,27 @@ timeout() { return $status } +# sanitize a variable +# $1 variable name +# $2 allowed shell pattern +# $3 disallowed shell pattern +sanitize_variable() { + local __VAR=$1 + eval __VALUE=\$$__VAR + local __ALLOWED=$2 + local __REJECT=$3 + + # removing all allowed should give empty string + if [ -n "$__ALLOWED" ]; then + [ -z "${__VALUE//$__ALLOWED}" ] || write_log 12 "sanitize on $__VAR found characters outside allowed subset" + fi + + # removing rejected pattern should give the same string as the input + if [ -n "$__REJECT" ]; then + [ "$__VALUE" = "${__VALUE//$__REJECT}" ] || write_log 12 "sanitize on $__VAR found rejected characters" + fi +} + # verify given host and port is connectable # $1 Host/IP to verify # $2 Port to verify @@ -508,7 +535,10 @@ verify_host_port() { __RUNPROG="$NSLOOKUP $__HOST >$DATFILE 2>$ERRFILE" fi write_log 7 "#> $__RUNPROG" - eval $__RUNPROG + ( + set -o noglob + eval $__RUNPROG + ) __ERR=$? # command error [ $__ERR -gt 0 ] && { @@ -561,7 +591,10 @@ verify_host_port() { if [ -n "$__NCEXT" ]; then # BusyBox nc compiled with extensions (timeout support) __RUNPROG="$__NC -w 1 $__IP $__PORT $DATFILE 2>$ERRFILE" write_log 7 "#> $__RUNPROG" - eval $__RUNPROG + ( + set -o noglob + eval $__RUNPROG + ) __ERR=$? [ $__ERR -eq 0 ] && return 0 write_log 3 "Connect error - BusyBox nc (netcat) Error '$__ERR'" @@ -570,7 +603,10 @@ verify_host_port() { else # nc compiled without extensions (no timeout support) __RUNPROG="timeout 2 -- $__NC $__IP $__PORT $DATFILE 2>$ERRFILE" write_log 7 "#> $__RUNPROG" - eval $__RUNPROG + ( + set -o noglob + eval $__RUNPROG + ) __ERR=$? [ $__ERR -eq 0 ] && return 0 write_log 3 "Connect error - BusyBox nc (netcat) timeout Error '$__ERR'" @@ -689,7 +725,7 @@ do_transfer() { local __BINDIP # set correct program to detect IP [ $use_ipv6 -eq 0 ] && __RUNPROG="network_get_ipaddr" || __RUNPROG="network_get_ipaddr6" - eval "$__RUNPROG __BINDIP $bind_network" || \ + ( set -o noglob ; eval "$__RUNPROG __BINDIP $bind_network" ) || \ write_log 13 "Can not detect local IP using '$__RUNPROG $bind_network' - Error: '$?'" write_log 7 "Force communication via IP '$__BINDIP'" __PROG="$__PROG --bind-address=$__BINDIP" @@ -815,7 +851,10 @@ do_transfer() { while : ; do write_log 7 "#> $__RUNPROG" - eval $__RUNPROG # DO transfer + ( + set -o noglob + eval $__RUNPROG # DO transfer + ) __ERR=$? # save error code [ $__ERR -eq 0 ] && return 0 # no error leave [ -n "$LUCI_HELPER" ] && return 1 # no retry if called by LuCI helper script @@ -900,7 +939,7 @@ get_local_ip () { network_flush_cache # force re-read data from ubus [ $use_ipv6 -eq 0 ] && __RUNPROG="network_get_ipaddr" \ || __RUNPROG="network_get_ipaddr6" - eval "$__RUNPROG __DATA $ip_network" || \ + ( set -o noglob ; eval "$__RUNPROG __DATA $ip_network" ) || \ write_log 13 "Can not detect local IP using $__RUNPROG '$ip_network' - Error: '$?'" [ -n "$__DATA" ] && write_log 7 "Local IP '$__DATA' detected on network '$ip_network'" elif [ -n "$ip_interface" ]; then @@ -984,7 +1023,10 @@ get_local_ip () { [ -n "$__DATA" ] && write_log 7 "Local IP '$__DATA' detected on interface '$ip_interface'" elif [ -n "$ip_script" ]; then write_log 7 "#> $ip_script >$DATFILE 2>$ERRFILE" - eval $ip_script >$DATFILE 2>$ERRFILE + ( + set -o noglob + eval $ip_script >$DATFILE 2>$ERRFILE + ) __ERR=$? if [ $__ERR -eq 0 ]; then __DATA=$(cat $DATFILE) @@ -1124,7 +1166,10 @@ get_registered_ip() { while : ; do write_log 7 "#> $__RUNPROG" - eval $__RUNPROG + ( + set -o noglob + eval $__RUNPROG + ) __ERR=$? if [ $__ERR -ne 0 ]; then write_log 3 "$__PROG error: '$__ERR'" diff --git a/net/ddns-scripts/files/dynamic_dns_updater.sh b/net/ddns-scripts/files/dynamic_dns_updater.sh index b2baae231b..5c73c12ff1 100755 --- a/net/ddns-scripts/files/dynamic_dns_updater.sh +++ b/net/ddns-scripts/files/dynamic_dns_updater.sh @@ -240,6 +240,15 @@ esac # without lookup host and possibly other required options we can do nothing for you [ -z "$lookup_host" ] && write_log 14 "Service section not configured correctly! Missing 'lookup_host'" +# verify validity of variables +[ -n "$lookup_host" ] && sanitize_variable lookup_host "$DNS_CHARSET" "" +[ -n "$dns_server" ] && sanitize_variable dns_server "$DNS_CHARSET" "" +[ -n "$domain" ] && sanitize_variable domain "$DNS_CHARSET" "" + +# Filter shell escape characters, if these are required in the URL, they +# can still be passed url encoded +[ -n "$param_opt" ] && sanitize_variable param_opt "" "$SHELL_ESCAPE" + [ -n "$update_url" ] && { # only check if update_url is given, update_scripts have to check themselves [ -z "$domain" ] && $(echo "$update_url" | grep "\[DOMAIN\]" >/dev/null 2>&1) && \ -- 2.30.2