[FLINK-37666] CWE-378: Creation of Temporary File With Insecure Permissions in Temporary File Creation - #28957
Open
Samrat002 wants to merge 1 commit into
Open
[FLINK-37666] CWE-378: Creation of Temporary File With Insecure Permissions in Temporary File Creation#28957Samrat002 wants to merge 1 commit into
Samrat002 wants to merge 1 commit into
Conversation
…ssions in Temporary File Creation
Collaborator
Contributor
Author
|
@rkhachatryan PTAL whenever time |
Comment on lines
+131
to
+132
| file = Files.createTempFile(directory.toPath(), CACHE_FILE_PREFIX, null).toFile(); | ||
| file.deleteOnExit(); |
Contributor
There was a problem hiding this comment.
Can we rather delete the file explicitly?
I'm concerned that the number of such files to delete will accumulate in a long running JVM process
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
File.createTempFile()creates temporary files with permissions determined by the OS umask, which on typical POSIX systems yields world-readable permissions (-rw-r--r--, 0644). This means any local user on the same host can read the file's contents for the lifetime of the temp file. Depending on the call site, those contents can be highly sensitive:The fix replaces File.createTempFile with java.nio.file.Files.createTempFile across all affected sites. On POSIX filesystems, Files.createTempFile applies owner-only permissions (-rw-------, 0600) atomically at creation time — no group or other access, and no read-then-chmod race window.
The security guarantee is POSIX-scoped (on Windows it falls back to directory ACLs, which are per-user by default), but Flink's production targets are POSIX, so the concern is fully addressed where it matters. Using the default-attribute form of Files.createTempFile (rather than passing explicit PosixFilePermissions) is intentional: explicit POSIX attributes throw UnsupportedOperationException on non-POSIX systems, making the default form the more portable choice.
Verifying the permission
Brief change log
Verifying this change
This change is a targeted security hardening / code cleanup. The security property (file permissions) is enforced by the JDK's NIO implementation and not exercised by existing Flink unit tests. No new test coverage is added, as the correct permissions can be verified by inspecting the POSIX attributes of the created file (as shown in the example above), and the surrounding logic is unchanged. Existing tests for the affected classes continue to exercise the same code paths and confirm no behavioral regression.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes / no) noDocumentation
Was generative AI tooling used to co-author this PR?
Generated-by: [Tool Name and Version]