Implemented strict mode to prevent insecure access to external variables

This commit is contained in:
Joseph Dalrymple 2023-04-04 20:52:56 -05:00
parent b01fc43580
commit fd44c4c146
10 changed files with 115 additions and 1 deletions

1
.gitignore vendored
View File

@ -2,6 +2,7 @@
diagnostic.partial
diagnostic.test
tests/*.diff
tests/*.actual
spec/
spec-runner/
node_modules/

View File

@ -118,6 +118,18 @@ There are more scripts available in the [demos directory](demo/) that could help
There are additional features that the program supports. Try using `mo --help` to see what is available.
Security
-----------
Because `mo` uses environment variables that could be unsafe in the event that `mo` is invoked programmatically with user-input, `mo` provides `--strict` mode when used with `--source=`. `mo` will attempt to track variables that are created by each `--source=`'ed environment file, and will disallow access to external variables.
To use this, `--strict` must be used with `--source=`:
```bash
./mo --strict --source=.env my-template.mustache
```
Concessions
-----------

55
mo
View File

@ -95,8 +95,15 @@
# treated as an empty value for the purposes of conditionals.
# MO_ORIGINAL_COMMAND - Used to find the `mo` program in order to generate a
# help message.
# MO_STRICT_MODE - Used to limit access to variables outside of sourced files
#
# Returns nothing.
MO_HAS_SOURCED=false
MO_STRICT_SWITCH=false
MO_STRICT_MODE=false
MO_SOURCED_VARS=()
mo() (
# This function executes in a subshell so IFS is reset.
# Namespace this variable so we don't conflict with desired values.
@ -119,6 +126,10 @@ mo() (
exit 0
;;
--strict)
MO_STRICT_SWITCH=true
;;
--allow-function-arguments)
# shellcheck disable=SC2030
MO_ALLOW_FUNCTION_ARGUMENTS=true
@ -147,8 +158,21 @@ mo() (
fi
if [[ -f "$f2source" ]]; then
MO_HAS_SOURCED=true
local BEFORE_VARS AFTER_VARS ALL_VARS
BEFORE_VARS=(`cat <(set -o posix ; set) | cut -d'=' -f1`)
# shellcheck disable=SC1090
. "$f2source"
AFTER_VARS=(`cat <(set -o posix ; set) | cut -d'=' -f1`)
ALL_VARS=( ${BEFORE_VARS[@]} ${AFTER_VARS[@]} )
MO_SOURCED_VARS=( \
${MO_SOURCED_VARS[@]} \
$(echo "${ALL_VARS[@]}" | tr ' ' '\n' | sort | uniq -u)\
)
unset BEFORE_VARS AFTER_VARS ALL_VARS
else
echo "No such file: $f2source" >&2
exit 1
@ -169,8 +193,17 @@ mo() (
done
fi
# Allow turning off Strict Mode
if [[ $MO_HAS_SOURCED == true ]] && [[ $MO_STRICT_SWITCH == true ]]; then
MO_STRICT_MODE=true
fi
moGetContent moContent "${files[@]}" || return 1
moParse "$moContent" "" true
for v in ${MO_SOURCED_VARS[@]}; do
unset -v $v > /dev/null 2>&1
done
)
@ -908,6 +941,7 @@ moShow() {
fi
fi
else
moTestLegal "${moNameParts[0]}" || return 1
# Further subindexes are disallowed
eval "echo -n \"\${${moNameParts[0]}[${moNameParts[1]%%.*}]}\""
fi
@ -989,6 +1023,18 @@ moStandaloneDenied() {
}
# Internal: Determines if a variable name is outside of the scope of the template
#
# Returns 0 if variable is safe, Returns 1 if variable is illegal
moTestLegal() {
[[ $MO_STRICT_MODE != true ]] && return 0
[[ " ${MO_SOURCED_VARS[*]} " =~ " ${1%.*} " ]] && return 0
echo "Illegal variable access $1" >&2
return 1
}
# Internal: Determines if the named thing is a function or if it is a
# non-empty environment variable. When MO_FALSE_IS_EMPTY is set to a
# non-empty value, then "false" is also treated is an empty value.
@ -1004,6 +1050,9 @@ moStandaloneDenied() {
# Returns 0 if the name is not empty, 1 otherwise. When MO_FALSE_IS_EMPTY
# is set, this returns 1 if the name is "false".
moTest() {
# Don't allow access to outside names
moTestLegal "$1" || return 1
# Test for functions
moIsFunction "$1" && return 0
@ -1030,7 +1079,11 @@ moTest() {
#
# Returns true (0) if the variable is set, 1 if the variable is unset.
moTestVarSet() {
[[ "${!1-a}" == "${!1-b}" ]]
# Don't allow access to outside names
moTestLegal "$1" || return 1
[[ "${!1-a}" == "${!1-b}" ]] && return 0
return 1
}

View File

@ -23,6 +23,9 @@ for TEST in tests/*.expected; do
. "${BASE}.env"
echo "Do not read this input" | mo "${BASE}.template"
fi
) | (
cat > "${BASE}.actual";
cat "${BASE}.actual"
) | diff -U5 - "${TEST}" > "${BASE}.diff"
statusCode=$?
@ -34,6 +37,7 @@ for TEST in tests/*.expected; do
echo "ok"
PASS=$(( PASS + 1 ))
rm "${BASE}.diff"
rm "${BASE}.actual"
fi
done

View File

@ -1,3 +1,4 @@
Purposely Unsafe for Backwards Compatibility
value
* 1
* 2

View File

@ -2,6 +2,7 @@
cd "${0%/*}" || exit 1
cat <<EOF | ../mo --source=source.vars
{{#BASH}}Purposely Unsafe for Backwards Compatibility{{/BASH}}
{{VAR}}
{{#ARR}}
* {{.}}

View File

@ -0,0 +1,2 @@
export A=from1
export B=from1

View File

@ -0,0 +1,3 @@
export B=from2
export C=from2
declare -a -x D=(from2)

View File

@ -0,0 +1,5 @@
A: from1
B: from2
C: from2
D: from2
D: from2

32
tests/strict-source-multiple.sh Executable file
View File

@ -0,0 +1,32 @@
#!/usr/bin/env bash
cd "${0%/*}" || exit 1
_CTRL_Z_=$'\cZ'
{
IFS=$'\n'"${_CTRL_Z_}" read -r -d "${_CTRL_Z_}" STDERR;
IFS=$'\n'"${_CTRL_Z_}" read -r -d "${_CTRL_Z_}" STDOUT;
} <<EOF
$((printf "${_CTRL_Z_}%s${_CTRL_Z_}%d${_CTRL_Z_}" "$(\
cat <<EOMO | ../mo --strict --source=strict-source-multiple-1.vars --source=strict-source-multiple-2.vars
{{#BASH_VERSINFO}}Illegal{{/BASH_VERSINFO}}
{{BASH_VERSINFO.0}}
A: {{A}}
B: {{B}}
C: {{C}}
D: {{D.0}}
D: {{#D}}{{.}}{{/D}}
EOMO
)" "${?}" 1>&2) 2>&1)
EOF
echo "${STDOUT[@]}"
expectedErr="Illegal variable access BASH_VERSINFO"
if [[ ! "$STDERR" =~ "$expectedErr" ]]; then
echo "STDERR should have contained an illegal variable access message:"
echo "$STDERR"
fi
exit 0