Do not eval OpenVPN foreign options as shell

AUTHORS

@@ -7,6 +7,7 @@
 # Individual Persons
 
 Alexandr Stelnykovych <alexandr.stelnykovych@ivpn.net>
+Sebastien Tardif <sebtardif@ncf.ca>
 
 
 # Organizations

daemon/References/Linux/etc/client.up

@@ -35,10 +35,12 @@ else
   # if we get no DOMAINS, then don't use either domain or search.
 
   while true; do
-    eval fopt=\$foreign_option_${i}
+    # Quote the expansion. A bare eval of the value runs command
+    # substitution from the server PUSH.
+    eval "fopt=\$(printf '%s' \"\${foreign_option_${i}}\")"
     [ -z "${fopt}" ] && break
 
-    case ${fopt} in
+    case "$fopt" in
       dhcp-option\ DOMAIN\ *)
             ndoms=$((ndoms + 1))
             domains="${domains} ${fopt#dhcp-option DOMAIN }"

daemon/vpn/openvpn/client_up_foreign_option_test.go

@@ -0,0 +1,45 @@
+package openvpn
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestClientUpDoesNotEvalForeignOption(t *testing.T) {
+	scriptPath := filepath.Join("..", "..", "References", "Linux", "etc", "client.up")
+	body, err := os.ReadFile(scriptPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	text := string(body)
+	if strings.Contains(text, "eval fopt=") {
+		t.Fatal("client.up still evals foreign_option without quoting")
+	}
+	if !strings.Contains(text, `eval "fopt=\$(printf '%s' \"\${foreign_option_${i}}\")"`) {
+		t.Fatal("client.up is missing the quoted foreign_option read")
+	}
+
+	out, err := exec.Command("sh", "-c", `
+i=1
+foreign_option_1='dhcp-option DNS $(echo PWNED)'
+marker=/tmp/ivpn-foreign-option-test-$$
+rm -f "$marker"
+eval "fopt=\$(printf '%s' \"\${foreign_option_${i}}\")"
+case "$fopt" in
+  dhcp-option\ DNS\ *)
+    printf '%s' "${fopt#dhcp-option DNS }"
+    ;;
+esac
+if [ -f "$marker" ]; then printf 'RAN'; fi
+`).Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := string(out)
+	if got != "$(echo PWNED)" {
+		t.Fatalf("parsed value = %q", got)
+	}
+}