Calling functions on script_type.

[bac0n]

Hi

Sanitize $script_type with "declare -f" makes it easy to inject own functions. One could easily exploit the script. ex. mycat(){ /bin/cat /etc/openvpn/credentials/password; };
export -f mycat;
export script_type=mycat; /etc/openvpn/update-systemd-resolved tun0

whole 'main' function is just bad.

[tomeon]

mycat(){ /bin/cat /etc/openvpn/credentials/password; }; 
export -f mycat; 
export script_type=mycat; /etc/openvpn/update-systemd-resolved tun0

Unless the user running this already has the ability to read /etc/openvpn/credentials/password, it is going to bail out with a permissions error:

$ sudo install -dm0700 --owner root --group root /etc/openvpn/credentials
$ sudo install -Dm0600 --owner root --group root /dev/null /etc/openvpn/credentials/password
$ sudo sh -c 'printf -- hunter2 > /etc/openvpn/credentials/password'
$ sudo ls -Al /etc/openvpn/credentials
$ mycat(){ /bin/cat /etc/openvpn/credentials/password; };
$ export -f mycat;
$ export script_type=mycat dev=nope0; /etc/openvpn/update-systemd-resolved mycat tun0
/bin/cat: /etc/openvpn/credentials/password: Permission denied

This escalation route might work if, for instance, a user had the ability to invoke update-systemd-resolved with sudo under a sudoers configuration that permits preserving the relevant environment variables. But, while I can't speak for the repo owner or the others who have contributed code, I doubt this is a recommended -- let alone supported -- configuration.

The prototypical use case is what's described in the README. Under this configuration, OpenVPN itself sets the script_type and dev variables (among others); see the "Environmental Variables" section of the OpenVPN manual. So unless an attacker can control what OpenVPN sets script_type to, this doesn't look like a promising vector.

---

whole 'main' function is just bad.

Could you elaborate on what you see as main's shortcomings?

---

N.B. there were a few issues with your example, like neither exporting the dev variable nor specifying a device as $1 when invoking update-systemd-resolved. That's what accounts for the differences between your example and the one I came up with to demonstrate the Permission denied error. I also had to change the mode bits on update-systemd-resolved to permit executing it as my normal user.

[bac0n]

It was just an example but the main issue is that you are using user input as decision without sanitation. The whole security concept of function overriding gets somewhat backwards. I think "main" is overcomplicated, for example "if ! read -r link if_index _ < <(get_link_info "$dev"); then" you are calling a function wrapped around a command that parsing and sets a variable and echoing it back to a process substitution that redirects it in to a command that sets the variable and tests the command, puh not sure i got it right.

#!/bin/bash

up(){ echo "up: $@" : }

down(){ echo "down: $@" : }

dev=${dev:-$1}; script_type=${script_type:-$2}

link=($(networkctl --no-pager --no-legend list "$dev" 2>&-))

if [[ ${#link[@]} -ne 5 ]]; then echo "Invalid device name." exit 0 fi

if [[ ${link[4]} != unmanaged ]]; then echo "Device is managed by systemd-networkd." exit 0 fi

case $script_type in up|down) $script_type $dev ${link[0]} "$@" ;; *) echo "Invalid script type." exit 0 ;; esac

exit 0