From aa0572b7c48b2652a9f86276bbbf3840939981af Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Fri, 16 Oct 2026 21:35:44 -0700 Subject: [PATCH] [Fix] `nvm_normalize_lts`, `nvm_ls_remote_index_tab`: keep untrusted text out of shell arithmetic and LTS aliases `nvm_normalize_lts` matched `lts/-N` with a glob that only pins the first digit after `lts/-`, then evaluated the rest inside `$((N+1))`. bash and zsh evaluate a variable's contents as an arithmetic expression there, including command substitutions in array subscripts, so a name like `lts/-10*path[$(cmd)1]` ran `cmd`. Such a name reaches this function from `nvm alias`, `nvm use`, `nvm install`, and `.nvmrc`, and also from LTS alias files written from a mirror's `index.tab`, which are resolved whenever `lts/*` or that codename is used. The version column of a mirror's `index.tab` was used verbatim: printed by `nvm ls-remote` and `nvm version-remote`, matched against patterns, and stored as the target of `lts/*` codename aliases. A hostile or compromised mirror could thus persist such a payload into `$NVM_DIR/alias/lts/`, where it is replayed later by `nvm alias` and alias resolution without the mirror being involved, and could put terminal escape sequences on the user's screen. `nvm_normalize_lts` now takes everything after `lts/-` with parameter expansion and passes the name through unchanged unless it is all digits, before any arithmetic happens. `lts/-1` through `lts/-N`, and the error when there are not that many LTS lines, behave as before; malformed offsets like `lts/-1x` or `lts/-10+1` are now returned unchanged in every shell, instead of erroring in dash or being evaluated in bash and zsh. This also drops the `$?` check after the arithmetic, which could never fire. `nvm_ls_remote_index_tab` now skips the header and keeps only rows whose version is `vX.Y.Z`, optionally followed by a `-` prerelease suffix of letters, digits, `.` and `-`, before anything is printed, stored, or matched. The check runs on the raw column, before the `iojs-` prefix is added. Every row of the current nodejs.org, nodejs.org nightly, and iojs.org index files passes, including `rc`, `nightly`, and `v8-canary` versions. Discovered by Anthropic's OSS Scanner, as vulnerabilities ANT-2026-0GXXE91V and ANT-2026-31ZIEY85. --- nvm.sh | 23 ++++---- .../Unit tests/nvm_ls_remote invalid versions | 53 ++++++++++++++++++ .../nvm_normalize_lts non-numeric offset | 56 +++++++++++++++++++ 3 files changed, 122 insertions(+), 10 deletions(-) create mode 100755 test/fast/Unit tests/nvm_ls_remote invalid versions create mode 100755 test/fast/Unit tests/nvm_normalize_lts non-numeric offset diff --git a/nvm.sh b/nvm.sh index edb50923..1f0f0564 100755 --- a/nvm.sh +++ b/nvm.sh @@ -1483,13 +1483,17 @@ nvm_normalize_lts() { case "${LTS}" in lts/-[123456789] | lts/-[123456789][0123456789]*) local N - N="$(echo "${LTS}" | cut -d '-' -f 2)" + N="${LTS#lts/-}" + # the glob above only pins the first digit; bash and zsh evaluate a variable's + # contents as an expression inside `$(( ))`, including command substitutions, + # so only a plain number may get there + case "${N}" in + *[!0123456789]*) + nvm_echo "${LTS}" + return 0 + ;; + esac N=$((N+1)) - # shellcheck disable=SC2181 - if [ $? -ne 0 ]; then - nvm_echo "${LTS}" - return 0 - fi local NVM_ALIAS_DIR NVM_ALIAS_DIR="$(nvm_alias_path)" local RESULT @@ -2742,11 +2746,10 @@ nvm_ls_remote_index_tab() { nvm_is_zsh && setopt local_options shwordsplit local VERSION_LIST + # the mirror's index.tab is untrusted: skip the header, and any row whose version + # is malformed, before it is printed, stored as an LTS alias, or matched against a pattern VERSION_LIST="$(nvm_download -L -s "${MIRROR}/index.tab" -o - \ - | command sed " - 1d; - s/^/${PREFIX}/; - " \ + | command awk -v prefix="${PREFIX}" 'NR > 1 && $1 ~ /^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$/ { print prefix $0 }' \ )" local LTS_ALIAS local LTS_VERSION diff --git a/test/fast/Unit tests/nvm_ls_remote invalid versions b/test/fast/Unit tests/nvm_ls_remote invalid versions new file mode 100755 index 00000000..27149af7 --- /dev/null +++ b/test/fast/Unit tests/nvm_ls_remote invalid versions @@ -0,0 +1,53 @@ +#!/bin/sh + +WORK="$PWD/ls_remote-invalid-versions-work.$$" +export HOME="$WORK/home" +export NVM_DIR="$HOME/.nvm" + +cleanup() { + unset -f die cleanup nvm_download + rm -rf "$WORK" +} +die () { echo "$@" ; cleanup ; exit 1; } + +mkdir -p "$NVM_DIR/alias" + +: nvm.sh +\. ../../../nvm.sh + +# a malicious/compromised mirror: the version field ($1) carries terminal escapes, +# shell syntax, or looks like an option; valid rows (including a prerelease) must still be listed +nvm_download() { + printf 'version\tdate\tfiles\tnpm\tv8\tuv\tzlib\topenssl\tmodules\tlts\tsecurity\n' + case "$*" in + *iojs.org*) return ;; + esac + printf 'v24.0.0-rc.1\t2026-01-01\tlinux-x64\t-\t-\t-\t-\t-\t-\t-\t-\n' + printf 'v23.0.0\033]0;pwned\007\033[1A\033[2K\t2026-01-01\tlinux-x64\t-\t-\t-\t-\t-\t-\tKrypton\t-\n' + printf '%s\t2026-01-01\tlinux-x64\t-\t-\t-\t-\t-\t-\tJod\t-\n' 'lts/-10*path[$(touch_pwned)1]' + printf '%s\t2026-01-01\tlinux-x64\t-\t-\t-\t-\t-\t-\t-\t-\n' '--v21.0.0' + printf 'v20.0.0\t2026-01-01\tlinux-x64\t-\t-\t-\t-\t-\t-\tIron\t-\n' +} + +OUTPUT="$(nvm_ls_remote)" +EXPECTED_OUTPUT='v20.0.0 Iron * +v24.0.0-rc.1' +[ "${OUTPUT}" = "${EXPECTED_OUTPUT}" ] || die "expected >${EXPECTED_OUTPUT}<, got >${OUTPUT}<" + +OUTPUT="$(nvm_remote_version 23)" +EXPECTED_OUTPUT='N/A' +[ "${OUTPUT}" = "${EXPECTED_OUTPUT}" ] || die "nvm_remote_version 23: expected >${EXPECTED_OUTPUT}<, got >${OUTPUT}<" + +[ ! -e "$NVM_DIR/alias/lts/krypton" ] || die 'a version with control characters was stored in lts/krypton' +[ ! -e "$NVM_DIR/alias/lts/jod" ] || die 'a version with shell syntax was stored in lts/jod' + +OUTPUT="$(cat "$NVM_DIR/alias/lts/*")" +EXPECTED_OUTPUT='lts/iron' +[ "${OUTPUT}" = "${EXPECTED_OUTPUT}" ] || die "expected lts/* to hold >${EXPECTED_OUTPUT}<, got >${OUTPUT}<" + +OUTPUT="$(cat "$NVM_DIR/alias/lts/iron")" +EXPECTED_OUTPUT='v20.0.0' +[ "${OUTPUT}" = "${EXPECTED_OUTPUT}" ] || die "expected lts/iron to hold >${EXPECTED_OUTPUT}<, got >${OUTPUT}<" + +cleanup +echo "nvm_ls_remote invalid versions: passed" diff --git a/test/fast/Unit tests/nvm_normalize_lts non-numeric offset b/test/fast/Unit tests/nvm_normalize_lts non-numeric offset new file mode 100755 index 00000000..419901ad --- /dev/null +++ b/test/fast/Unit tests/nvm_normalize_lts non-numeric offset @@ -0,0 +1,56 @@ +#!/bin/sh + +WORK="$PWD/normalize_lts-non-numeric-work.$$" +export HOME="$WORK/home" +export NVM_DIR="$HOME/.nvm" +MARKER='nvm_normalize_lts_pwned' + +cleanup() { + unset -f die cleanup + rm -rf "$WORK" + rm -f "${MARKER}" +} +die () { echo "$@" ; cleanup ; exit 1; } + +mkdir -p "$NVM_DIR/alias/lts" +rm -f "${MARKER}" + +: nvm.sh +\. ../../../nvm.sh + +echo 'lts/iron' > "$NVM_DIR/alias/lts/*" +echo 'v18.0.0' > "$NVM_DIR/alias/lts/hydrogen" +echo 'v20.0.0' > "$NVM_DIR/alias/lts/iron" + +ACTUAL="$(nvm_normalize_lts 'lts/-1')" +EXPECTED='lts/hydrogen' +[ "${ACTUAL}" = "${EXPECTED}" ] || die "lts/-1: expected >${EXPECTED}<, got >${ACTUAL}<" + +ACTUAL="$(nvm_normalize_lts 'lts/-1x')" +EXPECTED='lts/-1x' +[ "${ACTUAL}" = "${EXPECTED}" ] || die "lts/-1x: expected >${EXPECTED}<, got >${ACTUAL}<" + +ACTUAL="$(nvm_normalize_lts 'lts/-1-1')" +EXPECTED='lts/-1-1' +[ "${ACTUAL}" = "${EXPECTED}" ] || die "lts/-1-1: expected >${EXPECTED}<, got >${ACTUAL}<" + +ACTUAL="$(nvm_normalize_lts 'lts/-10+1')" +EXPECTED='lts/-10+1' +[ "${ACTUAL}" = "${EXPECTED}" ] || die "lts/-10+1: expected >${EXPECTED}<, got >${ACTUAL}<" + +# bash and zsh run command substitutions in array subscripts during arithmetic evaluation +INJECTED="lts/-10*path[\$(touch ${MARKER})1]" +ACTUAL="$(nvm_normalize_lts "${INJECTED}" 2>/dev/null)" +[ ! -e "${MARKER}" ] || die "nvm_normalize_lts evaluated ${INJECTED}" +[ "${ACTUAL}" = "${INJECTED}" ] || die "${INJECTED}: expected it unchanged, got >${ACTUAL}<" + +nvm_alias "${INJECTED}" >/dev/null 2>&1 +[ ! -e "${MARKER}" ] || die "nvm_alias evaluated ${INJECTED}" + +# an alias file written by an older nvm from a hostile mirror's index.tab +echo "${INJECTED}" > "$NVM_DIR/alias/lts/iron" +nvm_resolve_alias 'lts/*' >/dev/null 2>&1 +[ ! -e "${MARKER}" ] || die "nvm_resolve_alias evaluated a stored ${INJECTED}" + +cleanup +echo "nvm_normalize_lts non-numeric offset: passed"