javasecurity:S2083: show that we're validating paths to SonarCloud

SonarQube Cloud
,
1

In Java we are facing an issue similar to that one raised in Help SonarCloud with understanding the usage of untrusted and tainted input

We execute a validation on user-controlled variables before using the paths built from it but still SonarCloud complains about security:S2083.

Here is a code example

private Path filePath(final String fileName) { //fileName is user controlled
	Path parentPath = Paths.get(notUserControlledParameters);
	Path requestedFilePath = Paths.get(notUserControlledParameters, fileName);
	// security check against hacks like passing '../filename'
	if ( ! parentPath.equals( requestedFilePath.getParent() ) ) {
		throw new IllegalArgumentException( .... );
	}
	return requestedFilePath;
}

SonarCloud’s example how to fix this problem mentions FileUtils.directoryContains.

Is FileUtils.directoryContains the only sanitizer possible here for file operations or can others be used? In this particular case we cannot use it because the directory and the file might be not existing at the moment of this code execution.

Any advice is welcome!

1 Like
3

Hello and thanks for creating a new topic!

There are many ways to validate a path before using it, we have a quite extensive list of sanitizers and validators defined for it.
The way you validate your path seems secure to me. The path is not normalized but that should not matter in this case. Unfortunately, the way it is done our security engine is currently not able to detect that it is sufficient.

Is simply flagging the issue as a false-positive an option for you?

5

Hello @Hendrik_Buchwald!

It seems I managed to solve the issue by slightly changing the code to this one:

private Path filePath(final String fileName) { //fileName is user controlled
	Path parentPath = Paths.get(notUserControlledParameters);
	Path requestedFilePath = parentPath.resolve(fileName);
	// security check against hacks like passing '../filename'
	if ( ! parentPath.equals( requestedFilePath.getParent() ) ) {
		throw new IllegalArgumentException( .... );
	}
	return requestedFilePath;
}

I’ll double-check if SonarCloud doesn’t re-report it again…
In case of it returns back, false positive can be also considered as an option!
Just wondering: will the false positive marker stay for long?
Now this vulnerability seems to be based on multiple code statements in different lines (actually different files), so I worry if any of those lines would be changed, won’t the false positiveness marker be cleaned up and the issue resurrects.

6

Hello,

Yes, that is a nice solution. Path.resolve is defined as a sanitizer, so the return value of it is considered secure.

In general, issue reviews will persist. I am not 100% certain how the relations between issues in different branches are created. As far as I am aware it can happen that the connection is not found but it will require more than a few changes. So in general it should be a valid solution.

1 Like
7

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.