Ensure Path Traversal is fully addressed in MavenWrapperDownloader by hazendaz · Pull Request #363 · apache/maven-wrapper (original) (raw)

The original sanitizing patch is not enough to prevent path traversal. This approach more accurately deals with the security concern.

@hazendaz can you show a case where old method allow path traversal and new not
we cen extract it to simple method a use it in test

code is used to download

("$JAVA_HOME/bin/java" -cp .mvn/wrapper MavenWrapperDownloader "$wrapperUrl" "$wrapperJarPath") || rm -f "$wrapperJarPath"

and a variable wrapperJarPath is set in script by

wrapperJarPath="$MAVEN_PROJECTBASEDIR/.mvn/wrapper/maven-wrapper.jar"

it looks as constant, so issue is not critical for me

Hi @slawekjaranowski I agree its not critical, in fact its silly and only point is to shut up static scanners that trip on this file. As I have an override on top of maven wrapper, I'm ok that this didn't make the cut. I'll work on testing aspect so maybe this makes it next time. Snyk is also complaining about SSRF as well but all attempts we have made to silence that haven't worked out yet. In either case, any hacker that has access to this would have to also make the wrapper even fall back to this and they would have all the files to change behavior anyways so its not really an issue in that regard other than statically on code it is.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to make a static scanners happy it can be

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})