diff --git a/plugins/dotenv/.zunit.yml b/plugins/dotenv/.zunit.yml index ae65f8ef2..e5ea0c3a6 100644 --- a/plugins/dotenv/.zunit.yml +++ b/plugins/dotenv/.zunit.yml @@ -6,4 +6,4 @@ directories: time_limit: 0 fail_fast: false allow_risky: false -verbose: true +verbose: false diff --git a/plugins/dotenv/dotenv.plugin.zsh b/plugins/dotenv/dotenv.plugin.zsh index 7ba6a69d4..ee630481a 100644 --- a/plugins/dotenv/dotenv.plugin.zsh +++ b/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) diff --git a/plugins/dotenv/tests/_support/bootstrap b/plugins/dotenv/tests/_support/bootstrap index d4ccaf78c..495c9bd23 100644 --- a/plugins/dotenv/tests/_support/bootstrap +++ b/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 diff --git a/plugins/dotenv/tests/security.zunit b/plugins/dotenv/tests/security.zunit index 62f4e38a3..414f87bb7 100644 --- a/plugins/dotenv/tests/security.zunit +++ b/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" +}