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