java:S5443 Doubts about Path vs File

Rules and Languages Report False-positive / False-negative...
,
1
  • What language is this for? Java
  • Which rule? java:S5443
  • Why do you believe it’s a false-positive/false-negative? Both (?)
  • SonarQube Server / Community Build v25.3.0.104237 and v9.9
  • How can we reproduce the problem? Give us a self-contained snippet of code
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;

...

public Path downloadObject(AmazonS3 cosClient, String bucketName, String objectName) throws IOException {
		S3Object item = cosClient.getObject(new GetObjectRequest(bucketName, objectName));
		Path temporalPath = null;
		try (S3ObjectInputStream s3ois = item.getObjectContent()) {
			temporalPath = Files.createTempFile(Path.of("aaa"), null, null); // THIS LINE
			File temporalFile = temporalPath.toFile();
			boolean setReadable = temporalFile.setReadable(true, true);
			boolean setWritable = temporalFile.setWritable(true, true);
			if (!setReadable || !setWritable) {
				Log.warnv("...", setReadable, setWritable);
			}
			Files.copy(s3ois, temporalPath, StandardCopyOption.REPLACE_EXISTING);
			return temporalPath;
		} catch (IOException e) {
			throw e;
		}
	}
  1. Files.createTempFile(Path.of("aaa"), null, null); is not flagged (should be?). Path “aaa” is virtual, it doesn’t exist, yet.
  2. Files.createTempFile(null, null); is flagged, despite having setReadable and setWritable (setExecutable is not usable because there is no file created, only the path). With setExecutable it’s still flagged. The only difference with “Compliant Solution”, is that .toFile() is done in another line.
  3. “Compliant Solution” should be modified. Example triggers java:S899 because returned booleans are not used.

  • Context

    I download a file from an Object Storage, write it locally, work with it, and delete it.
    I just need the Path to copy the file from the Stream, using Files.copy.
    In case point 2 above is not a false positive, how should this be implemented?
    Point 1 feels like a false negative, given that it’s “predictable”.

5

Hi @gian1200,

Thanks for your detailed report on S5443. It is indeed a False Positive.

You’re correct that Files.createTempFile(null, null) being flagged despite permissions being set is a False Positive. We have an existing ticket for this issue, and it is a good reminder that we must tackle it.

Regarding Files.createTempFile(Path.of("aaa"), null, null) not being flagged: the rule is designed to assume that an explicitly provided directory is secure. This is a current limitation of the rule.

You can safely use any solutions provided in the compliant code section. I have created a ticket to modify the code example to prevent it from triggering S899.

Thank you! Your feedback is very helpful for refining our rules.

1 Like
8

Awesome, thanks!

Hopefully the ticket is not postponed again :sweat_smile:. It has been open since 2021.

1 Like