Copilot commented on code in PR #418:
URL: https://github.com/apache/maven-wrapper/pull/418#discussion_r3124409607
##########
maven-wrapper-distribution/src/resources/only-mvnw:
##########
@@ -160,8 +160,10 @@ case "${distributionUrl-}" in
*) die "distributionUrl is not valid, must match *-bin.zip or
maven-mvnd-*.zip, but found '${distributionUrl-}'" ;;
esac
-# prepare tmp dir
-if TMP_DOWNLOAD_DIR="$(mktemp -d)" && [ -d "$TMP_DOWNLOAD_DIR" ]; then
+# prepare tmp dir (POSIX-compatible: avoid mktemp which is not available in
all shells)
+_tmp_base="${TMPDIR:-/tmp}"
+TMP_DOWNLOAD_DIR="$_tmp_base/maven_wrapper.$$_$(date +%s)"
+if mkdir -p -- "$TMP_DOWNLOAD_DIR" && [ -d "$TMP_DOWNLOAD_DIR" ]; then
clean() { rm -rf -- "$TMP_DOWNLOAD_DIR"; }
Review Comment:
The temp directory path is predictable and `mkdir -p` will succeed if it
already exists, which can lead to reusing a pre-existing directory (DoS) and
enables TOCTOU/symlink attacks on files created inside it (e.g.,
`Downloader.java`, the downloaded archive). Prefer creating a *new* directory
with `mkdir` (no `-p`) in a loop until it succeeds, set a restrictive
`umask`/mode, and consider using `mktemp -d` when available with a POSIX
fallback when it isn’t.
##########
maven-wrapper-distribution/src/resources/only-mvnw:
##########
@@ -160,8 +160,10 @@ case "${distributionUrl-}" in
*) die "distributionUrl is not valid, must match *-bin.zip or
maven-mvnd-*.zip, but found '${distributionUrl-}'" ;;
esac
-# prepare tmp dir
-if TMP_DOWNLOAD_DIR="$(mktemp -d)" && [ -d "$TMP_DOWNLOAD_DIR" ]; then
+# prepare tmp dir (POSIX-compatible: avoid mktemp which is not available in
all shells)
+_tmp_base="${TMPDIR:-/tmp}"
+TMP_DOWNLOAD_DIR="$_tmp_base/maven_wrapper.$$_$(date +%s)"
+if mkdir -p -- "$TMP_DOWNLOAD_DIR" && [ -d "$TMP_DOWNLOAD_DIR" ]; then
Review Comment:
`date +%s` is not specified by POSIX and is missing on some platforms (incl.
AIX), so this temp dir name generation can fail or produce non-unique names.
Use a POSIX format (e.g., `date '+%Y%m%d%H%M%S'`) and/or a small looped suffix
counter to guarantee uniqueness without relying on `%s`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]