Skip to content

Navigation Menu

Sign in
Appearance settings
Sign in
Sign up
Appearance settings

Commit cbdf728

Browse filesBrowse the repository at this point in the historyBrowse files
nicolas-grekasnicolas-grekas
committed
bug symfony#65597 [Filesystem] Keep tempnam() files private when a suffix is given (iliaal)
This PR was merged into the 6.4 branch. Discussion ---------- [Filesystem] Keep tempnam() files private when a suffix is given | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT `Filesystem::tempnam()` has two branches. Without a suffix, and when the directory lives on the plain file scheme, it delegates to PHP's `tempnam()`, which always creates the file with mode `0600` whatever the process umask is. With a suffix, or on any other stream, it builds the name itself and creates the file with `fopen($tmpFile, 'x+')`. That call goes through `open(2)` with mode `0666`, so it honours the umask: under the common `0022` umask the file was created `0644`, and under a `0000` umask it was created `0666`. The two branches of the same method therefore did not offer the same guarantee, and the private one is the one the component relies on: `dumpFile()` writes the payload into the temporary file before relaxing its mode, so the content is exposed to every local user for as long as the file is wider than `0600`. This forces the umask to `0077` around the `fopen()` call and restores it right after. The file is private from the moment it exists, so there is no window where another process can open it, and no `chmod()` is needed. Custom stream wrappers benefit too: many do not implement `stream_metadata`, so a `chmod()` on them would have done nothing at all. `umask()` is the same mechanism the component already uses in `copy()` and `dumpFile()`, and the same pattern as `Console\Input\File\InputFile`. Reproduction under a `0022` umask: ```php $fs = new Filesystem(); printf("%o\n", fileperms($fs->tempnam(sys_get_temp_dir(), 'a')) & 0777); // 600, both before and after printf("%o\n", fileperms($fs->tempnam(sys_get_temp_dir(), 'b', '.txt')) & 0777); // 644 before, 600 after ``` Commits ------- f228697 [Filesystem] Keep tempnam() files private when a suffix is given
2 parents 1d8a783 + f228697 commit cbdf728
Copy full SHA for cbdf728

2 files changed

+27-1Lines changed: 27 additions & 1 deletion

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/Symfony/Component/Filesystem/Filesystem.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Component/Filesystem/Filesystem.php
+9-1Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,15 @@ public function tempnam(string $dir, string $prefix, string $suffix = ''): strin
650650

651651
// Use fopen instead of file_exists as some streams do not support stat
652652
// Use mode 'x+' to atomically check existence and create to avoid a TOCTOU vulnerability
653-
if (!$handle = self::box('fopen', $tmpFile, 'x+')) {
653+
// Force the umask so that the file is created private, as PHP's tempnam() does
654+
$umask = umask(0o077);
655+
try {
656+
$handle = self::box('fopen', $tmpFile, 'x+');
657+
} finally {
658+
umask($umask);
659+
}
660+
661+
if (!$handle) {
654662
continue;
655663
}
656664

Collapse file

‎src/Symfony/Component/Filesystem/Tests/FilesystemTest.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Component/Filesystem/Tests/FilesystemTest.php
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,6 +1494,24 @@ public function testTempnam()
14941494
$this->assertFileExists($filename);
14951495
}
14961496

1497+
public function testTempnamWithSuffixIsPrivate()
1498+
{
1499+
if ('\\' === \DIRECTORY_SEPARATOR) {
1500+
$this->markTestSkipped('This test cannot run on Windows.');
1501+
}
1502+
1503+
$oldUmask = umask(0o022);
1504+
try {
1505+
$filename = $this->filesystem->tempnam($this->workspace, 'foo', '.txt');
1506+
1507+
$this->assertFileExists($filename);
1508+
$this->assertSame(0o600, fileperms($filename) & 0o777);
1509+
$this->assertSame(0o022, umask());
1510+
} finally {
1511+
umask($oldUmask);
1512+
}
1513+
}
1514+
14971515
public function testTempnamTrimsTrailingWhitespaceFromTruncatedPrefix()
14981516
{
14991517
// PHP's tempnam() truncates the prefix to 63 characters; if that leaves a

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.