mirror of
https://github.com/nvm-sh/nvm.git
synced 2026-06-05 13:57:08 +08:00
[Fix] nvm_download: avoid eval so mirror-supplied version strings can't inject commands
`nvm_download` built a curl/wget command string and ran it with `eval`.
The download URLs embed the version string taken from the mirror's `index.tab`,
which is untrusted.
Wrapping each argument in double quotes inside the `eval` does not prevent command substitution,
so a version field such as `v1$(touch /tmp/proof)` was executed by the shell.
This bypassed the earlier quoting hardening in 0ce8f5a.
Pass every argument as a literal argv element instead of constructing a string for `eval`,
on both the curl and wget paths,
so URL arguments are never re-parsed by the shell.
The wget flag translation is now done per-argument with a POSIX
`set --` loop rather than `sed` over the joined string.
The auth header is sanitized and added once,
before invoking the downloader.
This commit is contained in:
Jordan Harband
93
nvm.sh
93
nvm.sh
@@ -115,47 +115,62 @@ nvm_get_latest() {
|
||||
nvm_echo "${NVM_LATEST_URL##*/}"
|
||||
}
|
||||
|
||||
# Every argument is passed through as a literal argv element so that untrusted,
|
||||
# mirror-supplied version strings in the URLs are never re-parsed by the shell
|
||||
# (which would allow command substitution / OS command injection).
|
||||
nvm_download() {
|
||||
if nvm_has "curl"; then
|
||||
local CURL_COMPRESSED_FLAG=""
|
||||
local CURL_HEADER_FLAG=""
|
||||
local sanitized_header
|
||||
|
||||
if [ -n "${NVM_AUTH_HEADER:-}" ]; then
|
||||
sanitized_header=$(nvm_sanitize_auth_header "${NVM_AUTH_HEADER}")
|
||||
CURL_HEADER_FLAG="--header \"Authorization: ${sanitized_header}\""
|
||||
fi
|
||||
|
||||
if nvm_curl_use_compression; then
|
||||
CURL_COMPRESSED_FLAG="--compressed"
|
||||
fi
|
||||
local NVM_DOWNLOAD_ARGS
|
||||
NVM_DOWNLOAD_ARGS=''
|
||||
for arg in "$@"; do
|
||||
NVM_DOWNLOAD_ARGS="${NVM_DOWNLOAD_ARGS} \"$arg\""
|
||||
done
|
||||
eval "curl -q --fail ${CURL_COMPRESSED_FLAG:-} ${CURL_HEADER_FLAG:-} ${NVM_DOWNLOAD_ARGS}"
|
||||
elif nvm_has "wget"; then
|
||||
# Emulate curl with wget
|
||||
ARGS=$(nvm_echo "$@" | command sed "
|
||||
s/--progress-bar /--progress=bar /
|
||||
s/--compressed //
|
||||
s/--fail //
|
||||
s/-L //
|
||||
s/-I /--server-response /
|
||||
s/-s /-q /
|
||||
s/-sS /-nv /
|
||||
s/-o /-O /
|
||||
s/-C - /-c /
|
||||
")
|
||||
|
||||
if [ -n "${NVM_AUTH_HEADER:-}" ]; then
|
||||
sanitized_header=$(nvm_sanitize_auth_header "${NVM_AUTH_HEADER}")
|
||||
ARGS="${ARGS} --header \"Authorization: ${sanitized_header}\""
|
||||
fi
|
||||
# shellcheck disable=SC2086
|
||||
eval wget $ARGS
|
||||
local sanitized_header
|
||||
sanitized_header=''
|
||||
if [ -n "${NVM_AUTH_HEADER:-}" ]; then
|
||||
sanitized_header="$(nvm_sanitize_auth_header "${NVM_AUTH_HEADER}")"
|
||||
fi
|
||||
|
||||
local NVM_DOWNLOADER
|
||||
NVM_DOWNLOADER=''
|
||||
if nvm_has "curl"; then
|
||||
NVM_DOWNLOADER='curl'
|
||||
set -- -q --fail "$@"
|
||||
if nvm_curl_use_compression; then
|
||||
set -- --compressed "$@"
|
||||
fi
|
||||
elif nvm_has "wget"; then
|
||||
NVM_DOWNLOADER='wget'
|
||||
# Emulate curl with wget
|
||||
local NVM_DOWNLOAD_WGET_COUNT
|
||||
NVM_DOWNLOAD_WGET_COUNT=$#
|
||||
local NVM_DOWNLOAD_WGET_SKIP
|
||||
NVM_DOWNLOAD_WGET_SKIP=0
|
||||
local NVM_DOWNLOAD_WGET_ARG
|
||||
for NVM_DOWNLOAD_WGET_ARG in "$@"; do
|
||||
if [ "${NVM_DOWNLOAD_WGET_SKIP}" = '1' ]; then
|
||||
NVM_DOWNLOAD_WGET_SKIP=0
|
||||
continue
|
||||
fi
|
||||
case "${NVM_DOWNLOAD_WGET_ARG}" in
|
||||
'--progress-bar') set -- "$@" '--progress=bar' ;;
|
||||
'--compressed') : ;;
|
||||
'--fail') : ;;
|
||||
'-L') : ;;
|
||||
'-I') set -- "$@" '--server-response' ;;
|
||||
'-s') set -- "$@" '-q' ;;
|
||||
'-sS') set -- "$@" '-nv' ;;
|
||||
'-o') set -- "$@" '-O' ;;
|
||||
'-C') NVM_DOWNLOAD_WGET_SKIP=1; set -- "$@" '-c' ;;
|
||||
*) set -- "$@" "${NVM_DOWNLOAD_WGET_ARG}" ;;
|
||||
esac
|
||||
done
|
||||
shift "${NVM_DOWNLOAD_WGET_COUNT}"
|
||||
fi
|
||||
|
||||
if [ -z "${NVM_DOWNLOADER}" ]; then
|
||||
return 0
|
||||
fi
|
||||
|
||||
if [ -n "${NVM_AUTH_HEADER:-}" ]; then
|
||||
set -- "$@" --header "Authorization: ${sanitized_header}"
|
||||
fi
|
||||
|
||||
"${NVM_DOWNLOADER}" "$@"
|
||||
}
|
||||
|
||||
nvm_sanitize_auth_header() {
|
||||
|
||||
86
test/fast/Unit tests/nvm_download no eval injection
Executable file
86
test/fast/Unit tests/nvm_download no eval injection
Executable file
@@ -0,0 +1,86 @@
|
||||
#!/bin/sh
|
||||
|
||||
OLDPATH="$PATH"
|
||||
WORK="$PWD/nvm_download-noeval-work.$$"
|
||||
TEST_BIN="$WORK/bin"
|
||||
ARGV_LOG="$WORK/argv.log"
|
||||
PROOF="$WORK/nvm_injection_proof"
|
||||
|
||||
cleanup() {
|
||||
unset -f die cleanup
|
||||
rm -rf "$WORK"
|
||||
export PATH="$OLDPATH"
|
||||
}
|
||||
die () { echo "$@" ; cleanup ; exit 1; }
|
||||
|
||||
\. ../../../nvm.sh
|
||||
|
||||
OLDPATH="$PATH"
|
||||
|
||||
mkdir -p "$TEST_BIN"
|
||||
|
||||
# fake curl/wget: record each received argument verbatim, then succeed
|
||||
{
|
||||
echo '#!/bin/sh'
|
||||
echo ': > "$ARGV_LOG"'
|
||||
echo 'for a in "$@"; do printf "%s\n" "$a" >> "$ARGV_LOG"; done'
|
||||
echo 'exit 0'
|
||||
} > "$TEST_BIN/curl"
|
||||
chmod +x "$TEST_BIN/curl"
|
||||
cp "$TEST_BIN/curl" "$TEST_BIN/wget"
|
||||
|
||||
export ARGV_LOG
|
||||
export PATH="$TEST_BIN:$OLDPATH"
|
||||
|
||||
# given a url containing command-substitution syntax (mirror-supplied)
|
||||
INJECT_URL="http://example.test/v1\$(touch ${PROOF})/x.tar.gz"
|
||||
|
||||
# when nvm_download is invoked (curl path)
|
||||
rm -f "$PROOF"
|
||||
nvm_download "$INJECT_URL" -o - || die 'nvm_download (curl) returned nonzero on injection url'
|
||||
# then the substitution must not have executed
|
||||
[ ! -e "$PROOF" ] || die "command injection fired via curl path: proof file was created"
|
||||
# and curl must have received the url as one literal argument
|
||||
grep -Fxq "$INJECT_URL" "$ARGV_LOG" || die "curl did not receive the url as a single literal argument; got: $(cat "$ARGV_LOG")"
|
||||
|
||||
# given a normal download (curl path)
|
||||
URL="https://nodejs.org/dist/index.tab"
|
||||
FILE="$WORK/target"
|
||||
# when invoked with -L -s URL -o FILE
|
||||
nvm_download -L -s "$URL" -o "$FILE" || die 'nvm_download (curl) returned nonzero on normal url'
|
||||
# then the url, -o, and output file are passed through verbatim
|
||||
grep -Fxq "$URL" "$ARGV_LOG" || die "curl did not receive the url; got: $(cat "$ARGV_LOG")"
|
||||
grep -Fxqe "-o" "$ARGV_LOG" || die "curl did not receive -o; got: $(cat "$ARGV_LOG")"
|
||||
grep -Fxq "$FILE" "$ARGV_LOG" || die "curl did not receive the output file; got: $(cat "$ARGV_LOG")"
|
||||
# and curl-style flags are passed through verbatim (curl keeps -s; it is not translated to wget's -q)
|
||||
grep -Fxqe "-s" "$ARGV_LOG" || die "curl did not receive -s verbatim; got: $(cat "$ARGV_LOG")"
|
||||
rm -f "$FILE"
|
||||
|
||||
# given curl is unavailable (the wget-path calls run with PATH limited to our
|
||||
# fake wget, so neither the fake nor the system curl is found)
|
||||
rm -f "$TEST_BIN/curl"
|
||||
|
||||
# when nvm_download is invoked with the injection url (wget path)
|
||||
rm -f "$PROOF"
|
||||
( PATH="$TEST_BIN"; export PATH; nvm_download "$INJECT_URL" -o - ) || die 'nvm_download (wget) returned nonzero on injection url'
|
||||
# then the substitution must not have executed
|
||||
[ ! -e "$PROOF" ] || die "command injection fired via wget path: proof file was created"
|
||||
grep -Fxq "$INJECT_URL" "$ARGV_LOG" || die "wget did not receive the url as a single literal argument; got: $(cat "$ARGV_LOG")"
|
||||
|
||||
# when invoked with -L -C - --progress-bar URL -o FILE (wget path)
|
||||
( PATH="$TEST_BIN"; export PATH; nvm_download -L -C - --progress-bar "$URL" -o "$FILE" ) || die 'nvm_download (wget) returned nonzero on normal url'
|
||||
# then flags are translated to wget equivalents
|
||||
grep -Fxqe "-c" "$ARGV_LOG" || die "wget did not translate -C - to -c; got: $(cat "$ARGV_LOG")"
|
||||
grep -Fxqe "--progress=bar" "$ARGV_LOG" || die "wget did not translate --progress-bar; got: $(cat "$ARGV_LOG")"
|
||||
grep -Fxqe "-O" "$ARGV_LOG" || die "wget did not translate -o to -O; got: $(cat "$ARGV_LOG")"
|
||||
grep -Fxqe "-L" "$ARGV_LOG" && die "wget should drop -L; got: $(cat "$ARGV_LOG")"
|
||||
grep -Fxqe "-C" "$ARGV_LOG" && die "wget should not pass -C through; got: $(cat "$ARGV_LOG")"
|
||||
grep -Fxqe "-" "$ARGV_LOG" && die "wget should drop the lone - after -C; got: $(cat "$ARGV_LOG")"
|
||||
|
||||
# when invoked with -s (wget path)
|
||||
( PATH="$TEST_BIN"; export PATH; nvm_download -L -s "$URL" -o "$FILE" ) || die 'nvm_download (wget) returned nonzero on -s url'
|
||||
# then -s becomes -q
|
||||
grep -Fxqe "-q" "$ARGV_LOG" || die "wget did not translate -s to -q; got: $(cat "$ARGV_LOG")"
|
||||
|
||||
cleanup
|
||||
echo "nvm_download no eval injection: passed"
|
||||
Reference in New Issue
Block a user