diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index be0729a3929d0..caac4904706df 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -3194,12 +3194,6 @@
-
-
-
-
-
-
diff --git a/lib/private/TempManager.php b/lib/private/TempManager.php
index 0df31dce3fff1..31ae8181df94a 100644
--- a/lib/private/TempManager.php
+++ b/lib/private/TempManager.php
@@ -34,6 +34,7 @@
use bantu\IniGetWrapper\IniGetWrapper;
use OCP\IConfig;
use OCP\ITempManager;
+use OCP\Security\ISecureRandom;
use Psr\Log\LoggerInterface;
class TempManager implements ITempManager {
@@ -58,51 +59,25 @@ public function __construct(LoggerInterface $logger, IConfig $config, IniGetWrap
$this->tmpBaseDir = $this->getTempBaseDir();
}
- /**
- * Builds the filename with suffix and removes potential dangerous characters
- * such as directory separators.
- *
- * @param string $absolutePath Absolute path to the file / folder
- * @param string $postFix Postfix appended to the temporary file name, may be user controlled
- * @return string
- */
- private function buildFileNameWithSuffix($absolutePath, $postFix = '') {
+ private function generateTemporaryPath(string $postFix): string {
+ $secureRandom = \OCP\Server::get(ISecureRandom::class);
+ $absolutePath = $this->tmpBaseDir . '/' . self::TMP_PREFIX . $secureRandom->generate(32, ISecureRandom::CHAR_ALPHANUMERIC);
+
if ($postFix !== '') {
$postFix = '.' . ltrim($postFix, '.');
$postFix = str_replace(['\\', '/'], '', $postFix);
- $absolutePath .= '-';
}
return $absolutePath . $postFix;
}
- /**
- * Create a temporary file and return the path
- *
- * @param string $postFix Postfix appended to the temporary file name
- * @return string
- */
- public function getTemporaryFile($postFix = '') {
- if (is_writable($this->tmpBaseDir)) {
- // To create an unique file and prevent the risk of race conditions
- // or duplicated temporary files by other means such as collisions
- // we need to create the file using `tempnam` and append a possible
- // postfix to it later
- $file = tempnam($this->tmpBaseDir, self::TMP_PREFIX);
- $this->current[] = $file;
+ public function getTemporaryFile($postFix = ''): string|false {
+ $path = $this->generateTemporaryPath($postFix);
- // If a postfix got specified sanitize it and create a postfixed
- // temporary file
- if ($postFix !== '') {
- $fileNameWithPostfix = $this->buildFileNameWithSuffix($file, $postFix);
- touch($fileNameWithPostfix);
- chmod($fileNameWithPostfix, 0600);
- $this->current[] = $fileNameWithPostfix;
- return $fileNameWithPostfix;
- }
-
- return $file;
- } else {
+ $old_umask = umask(0077);
+ $fp = fopen($path, 'x');
+ umask($old_umask);
+ if ($fp === false) {
$this->log->warning(
'Can not create a temporary file in directory {dir}. Check it exists and has correct permissions',
[
@@ -111,30 +86,16 @@ public function getTemporaryFile($postFix = '') {
);
return false;
}
- }
- /**
- * Create a temporary folder and return the path
- *
- * @param string $postFix Postfix appended to the temporary folder name
- * @return string
- */
- public function getTemporaryFolder($postFix = '') {
- if (is_writable($this->tmpBaseDir)) {
- // To create an unique directory and prevent the risk of race conditions
- // or duplicated temporary files by other means such as collisions
- // we need to create the file using `tempnam` and append a possible
- // postfix to it later
- $uniqueFileName = tempnam($this->tmpBaseDir, self::TMP_PREFIX);
- $this->current[] = $uniqueFileName;
+ fclose($fp);
+ $this->current[] = $path;
+ return $path;
+ }
- // Build a name without postfix
- $path = $this->buildFileNameWithSuffix($uniqueFileName . '-folder', $postFix);
- mkdir($path, 0700);
- $this->current[] = $path;
+ public function getTemporaryFolder($postFix = ''): string|false {
+ $path = $this->generateTemporaryPath($postFix) . '/';
- return $path . '/';
- } else {
+ if (mkdir($path, 0700) === false) {
$this->log->warning(
'Can not create a temporary folder in directory {dir}. Check it exists and has correct permissions',
[
@@ -143,6 +104,9 @@ public function getTemporaryFolder($postFix = '') {
);
return false;
}
+
+ $this->current[] = $path;
+ return $path;
}
/**
diff --git a/lib/public/ITempManager.php b/lib/public/ITempManager.php
index f594bad3992e5..bca8eb63b5855 100644
--- a/lib/public/ITempManager.php
+++ b/lib/public/ITempManager.php
@@ -32,20 +32,20 @@ interface ITempManager {
/**
* Create a temporary file and return the path
*
- * @param string $postFix
- * @return string
+ * @param string $postFix Postfix appended to the temporary file name
+ *
* @since 8.0.0
*/
- public function getTemporaryFile($postFix = '');
+ public function getTemporaryFile(string $postFix = ''): string|false;
/**
* Create a temporary folder and return the path
*
- * @param string $postFix
- * @return string
+ * @param string $postFix Postfix appended to the temporary folder name
+ *
* @since 8.0.0
*/
- public function getTemporaryFolder($postFix = '');
+ public function getTemporaryFolder(string $postFix = ''): string|false;
/**
* Remove the temporary files and folders generated during this request
diff --git a/tests/lib/TempManagerTest.php b/tests/lib/TempManagerTest.php
index 4b342c365b595..e6a862921a78a 100644
--- a/tests/lib/TempManagerTest.php
+++ b/tests/lib/TempManagerTest.php
@@ -155,34 +155,23 @@ public function testLogCantCreateFolder() {
$this->assertFalse($manager->getTemporaryFolder());
}
- public function testBuildFileNameWithPostfix() {
+ public function testGenerateTemporaryPathWithPostfix(): void {
$logger = $this->createMock(LoggerInterface::class);
$tmpManager = self::invokePrivate(
$this->getManager($logger),
- 'buildFileNameWithSuffix',
- ['/tmp/myTemporaryFile', 'postfix']
+ 'generateTemporaryPath',
+ ['postfix']
);
- $this->assertEquals('/tmp/myTemporaryFile-.postfix', $tmpManager);
+ $this->assertStringEndsWith('.postfix', $tmpManager);
}
- public function testBuildFileNameWithoutPostfix() {
+ public function testGenerateTemporaryPathTraversal(): void {
$logger = $this->createMock(LoggerInterface::class);
$tmpManager = self::invokePrivate(
$this->getManager($logger),
- 'buildFileNameWithSuffix',
- ['/tmp/myTemporaryFile', '']
- );
-
- $this->assertEquals('/tmp/myTemporaryFile', $tmpManager);
- }
-
- public function testBuildFileNameWithSuffixPathTraversal() {
- $logger = $this->createMock(LoggerInterface::class);
- $tmpManager = self::invokePrivate(
- $this->getManager($logger),
- 'buildFileNameWithSuffix',
- ['foo', '../Traversal\\../FileName']
+ 'generateTemporaryPath',
+ ['../Traversal\\../FileName']
);
$this->assertStringEndsNotWith('./Traversal\\../FileName', $tmpManager);