fix(dotenv): forbid setting special variables

plugins/dotenv/.zunit.yml

@@ -6,4 +6,4 @@ directories:
 time_limit: 0
 fail_fast: false
 allow_risky: false
-verbose: true
+verbose: false

plugins/dotenv/dotenv.plugin.zsh

@@ -7,7 +7,6 @@
 : ${ZSH_DOTENV_ALLOWED_LIST:="${ZSH_CACHE_DIR:-$ZSH/cache}/dotenv-allowed.list"}
 : ${ZSH_DOTENV_DISALLOWED_LIST:="${ZSH_CACHE_DIR:-$ZSH/cache}/dotenv-disallowed.list"}
 
-
 ## Functions
 
 parse_dotenv() {
@@ -55,7 +54,6 @@ parse_dotenv() {
   #   VAR1=value1; VAR2=value2
   #   VAR3="multi
   #   line value"
-  #
   # Result:
   #   typeset -a nodes=( 'VAR1=value1' ';' 'VAR2=value2' ';' $'VAR3="multi\nline value"' )
   #   typeset -a lines=( 'VAR1=value1' 'VAR2=value2' $'VAR3="multi\nline value"' )
@@ -73,7 +71,33 @@ parse_dotenv() {
     [[ -z "$line" ]] || line+=" "
     line="$node"
   done
-  # typeset -p nodes lines
+
+  local -a forbidden_vars=(
+    NODE_OPTIONS
+    BASH_ENV
+    ENV
+    ZDOTDIR
+    ZSH
+    LD_PRELOAD
+    LD_LIBRARY_PATH
+    DYLD_INSERT_LIBRARIES
+    GIT_CONFIG_GLOBAL
+    GIT_DIR
+    GIT_EDITOR
+    GIT_EXTERNAL_DIFF
+    GIT_EXEC_PATH
+    GIT_PAGER
+    GIT_SSH
+    GIT_SSH_COMMAND
+    GIT_SSL_NO_VERIFY
+    GIT_TEMPLATE_DIR
+    VISUAL
+    PAGER
+    EDITOR
+    ${(k)parameters[(R)*export*special]}
+  )
+  local forbidden="${(j:|:)forbidden_vars}"
+
 
   # Each line contains a single command line, we need to parse valid KEY=VALUE pairs
   for line in "${lines[@]}"; do
@@ -90,6 +114,11 @@ parse_dotenv() {
     key="${match[1]}"
     value="${match[2]}"
 
+    # Filter out variables to be ignored for security reasons (best effort)
+    if [[ "$key" == (${~forbidden}) ]]; then
+      continue
+    fi
+
     # Use tokenization to split value with native shell parsing (handles quotes and escapes)
     # Ignore any values that parse to multiple words, e.g. `BASE_URL=/ echo command run`
     local -a words
@@ -102,12 +131,12 @@ parse_dotenv() {
     #
     # Filter lines with command expansion not in safe contexts
     #
-    # Reader's note: this is actually a "best effort" check (works in tests), but
+    # READER'S NOTE: this is actually a "best effort" check (works in tests), but
     # only to prevent setting variables with command substitution. The actual effect
     # of setting them would not be a vulnerability, because we use `typeset name=value`
     # and value is a quoted string parsed by zsh itself with `${(Z:C:)content}`.
     #
-    # What does this mean? If we remove remove this filter block, this is what happens:
+    # What does this mean? If we were to remove this filter block, this is what would happen:
     #
     # Input: DANGEROUS=$(echo this is a command)
     # Output: DANGEROUS='$(echo this is a command)' (literal string, no command execution)

plugins/dotenv/tests/_support/bootstrap

@@ -104,10 +104,10 @@ _zunit_assert_var_same_as() {
     for key in "${keys[@]}"; do
       # Key match checks
       if [[ -v "value[$key]" && ! -v "comparison[$key]" ]]; then
-        echo "'$1[$key]' is set"
+        echo "'$1[$key]' is set (value='${value[$key]}')"
         ret=1
       elif [[ ! -v "value[$key]" && -v "comparison[$key]" ]]; then
-        echo "'$1[$key]' is not set"
+        echo "'$1[$key]' is not set (expected='${comparison[$key]}')"
         ret=1
       # Value match checks
       elif [[ "${value[$key]}" != "${comparison[$key]}" ]]; then

plugins/dotenv/tests/security.zunit

@@ -162,3 +162,48 @@ EOF
 
   assert "DOTENV_TEST_VARS" var_same_as "expected_vars"
 }
+
+
+
+@test 'blocks changes of special environment variables' {
+  _parse_dotenv_test =(<<'EOF'
+# Executes on the next node/npm/npx invocation
+NODE_OPTIONS=--require=./payload.js
+
+# Used for shell initialization
+BASH_ENV=./payload.sh
+# Used for shell initialization in zsh, but also respected by some tools like git
+# - https://man7.org/linux/man-pages/man1/dash.1.html#DESCRIPTION:~:text=by%20the%20shell.-,Invocation,-If%20no%20args
+# - https://zsh.sourceforge.io/Doc/Release/Parameters.html#index-ENV
+ENV=./payload.sh
+# Used for zsh startup
+ZDOTDIR=./.malicious_zsh
+ZSH=./.malicious_zsh
+
+# These are used for native code injection
+LD_PRELOAD=./payload.so
+LD_LIBRARY_PATH=./malicious_libs
+DYLD_INSERT_LIBRARIES=./payload.dylib
+
+# Git environment variables
+GIT_CONFIG_GLOBAL=./.gitconfig-malicious
+GIT_DIR=./malicious_git_dir
+GIT_EDITOR=./malicious_editor
+GIT_EXTERNAL_DIFF=./malicious_diff
+GIT_EXEC_PATH=./.malicious_git_exec
+GIT_PAGER=./malicious_pager
+GIT_SSH=./malicious_ssh
+GIT_SSH_COMMAND=./malicious_ssh_command
+GIT_SSL_NO_VERIFY=true
+GIT_TEMPLATE_DIR=./malicious_templates # for persistence
+
+# Special exported variables
+PATH=./malicious_bin:$PATH
+EDITOR=./malicious
+VISUAL=./malicious
+PAGER=./malicious
+EOF
+)
+
+  assert "DOTENV_TEST_VARS" var_same_as "expected_vars"
+}