Skip to content

Only attempt to create tempDir (and parents) when the last level of it is not a symlink#3511

Merged
katzyn merged 2 commits intoh2database:masterfrom
aikebah:issue-3510
May 19, 2022
Merged

Only attempt to create tempDir (and parents) when the last level of it is not a symlink#3511
katzyn merged 2 commits intoh2database:masterfrom
aikebah:issue-3510

Conversation

@aikebah
Copy link
Contributor

@aikebah aikebah commented May 18, 2022

First check whether the java.io.tmpdir is a symlink before trying to create the directory structure including any missing parents as by design Files.createDirectories() will throw a FileAlreadyExistsException when the last level of the path is a symlink.

Fixes #3510

if (!Files.isSymbolicLink(tempDir)) {
Files.createDirectories(tempDir);
}
file = Files.createTempFile(prefix, suffix);
Copy link
Contributor

Choose a reason for hiding this comment

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

PATH_TRAVERSAL_IN: This API (java/nio/file/Files.createTempFile(Ljava/lang/String;Ljava/lang/String;[Ljava/nio/file/attribute/FileAttribute;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The PATH_TRAVERSAL_IN behaviour is unmodified by this change, so won't fix it in my PR

@@ -445,7 +445,10 @@ public FilePath createTempFile(String suffix, boolean inTempDir) throws IOExcept
Path file = Paths.get(name + '.').toAbsolutePath();
Copy link
Contributor

Choose a reason for hiding this comment

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

PATH_TRAVERSAL_IN: This API (java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@katzyn
Copy link
Contributor

katzyn commented May 18, 2022

Thank you for your contribution!

I think it will be better to perform Files.isDirectory() check. Without LinkOption.NOFOLLOW_LINKS it should be able to detect symbolic links to directories properly.

Files.isSymbolicLink() returns true for all symbolic links, but we need to make sure that it is a directory or link to directory. Symbolic link can point to other objects or even point to nowhere. It also may not work if some parent directory in the path is a symbolic link too.

@aikebah
Copy link
Contributor Author

aikebah commented May 18, 2022

I think it will be better to perform Files.isDirectory() check. Without LinkOption.NOFOLLOW_LINKS it should be able to detect symbolic links to directories properly.

Agree, didn't test it before, blindly assumed isDirectory would suffer from similar symptoms, but it's indeed capable of detecting the symlinked directory as a directory. Updated the PR for it.

if (!Files.isDirectory(tempDir)) {
Files.createDirectories(tempDir);
}
file = Files.createTempFile(prefix, suffix);
Copy link
Contributor

Choose a reason for hiding this comment

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

PATH_TRAVERSAL_IN: This API (java/nio/file/Files.createTempFile(Ljava/lang/String;Ljava/lang/String;[Ljava/nio/file/attribute/FileAttribute;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -445,7 +445,10 @@ public FilePath createTempFile(String suffix, boolean inTempDir) throws IOExcept
Path file = Paths.get(name + '.').toAbsolutePath();
Copy link
Contributor

Choose a reason for hiding this comment

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

PATH_TRAVERSAL_IN: This API (java/nio/file/Paths.get(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;) reads a file whose location might be specified by user input

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@katzyn katzyn merged commit 210aabf into h2database:master May 19, 2022
@katzyn
Copy link
Contributor

katzyn commented May 19, 2022

Thanks!

@aikebah aikebah deleted the issue-3510 branch May 19, 2022 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PreparedStatement execution with java.io.tmpdir pointing to a directory symlink results in FileAlreadyExistsException

2 participants