server/lib/private/tempmanager.php

193 lines
5.2 KiB
PHP
Raw Normal View History

<?php
/**
2015-03-26 10:44:34 +00:00
* @author Morris Jobke <hey@morrisjobke.de>
* @author Robin Appelman <icewind@owncloud.com>
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
* @author Lukas Reschke <lukas@owncloud.com>
2015-03-26 10:44:34 +00:00
*
* @copyright Copyright (c) 2015, ownCloud, Inc.
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OC;
use OCP\ILogger;
use OCP\ITempManager;
class TempManager implements ITempManager {
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
/** @var string[] Current temporary files and folders, used for cleanup */
protected $current = [];
/** @var string i.e. /tmp on linux systems */
protected $tmpBaseDir;
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
/** @var ILogger */
protected $log;
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
/** Prefix */
const TMP_PREFIX = 'oc_tmp_';
/**
* @param string $baseDir
* @param \OCP\ILogger $logger
*/
public function __construct($baseDir, ILogger $logger) {
$this->tmpBaseDir = $baseDir;
$this->log = $logger;
}
/**
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
* 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
*/
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
private function buildFileNameWithSuffix($absolutePath, $postFix = '') {
if($postFix !== '') {
$postFix = '.' . ltrim($postFix, '.');
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
$postFix = str_replace(['\\', '/'], '', $postFix);
$absolutePath .= '-';
}
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
return $absolutePath . $postFix;
}
/**
* Create a temporary file and return the path
*
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
* @param string $postFix Postfix appended to the temporary file name
* @return string
*/
public function getTemporaryFile($postFix = '') {
if (is_writable($this->tmpBaseDir)) {
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
// 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;
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
// 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 {
$this->log->warning(
'Can not create a temporary file in directory {dir}. Check it exists and has correct permissions',
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
[
'dir' => $this->tmpBaseDir,
]
);
return false;
}
}
/**
* Create a temporary folder and return the path
*
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
* @param string $postFix Postfix appended to the temporary folder name
* @return string
*/
public function getTemporaryFolder($postFix = '') {
if (is_writable($this->tmpBaseDir)) {
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
// 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;
// Build a name without postfix
$path = $this->buildFileNameWithSuffix($uniqueFileName . '-folder', $postFix);
mkdir($path, 0700);
$this->current[] = $path;
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
return $path . '/';
} else {
$this->log->warning(
'Can not create a temporary folder in directory {dir}. Check it exists and has correct permissions',
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
[
'dir' => $this->tmpBaseDir,
]
);
return false;
}
}
/**
* Remove the temporary files and folders generated during this request
*/
public function clean() {
$this->cleanFiles($this->current);
}
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
/**
* @param string[] $files
*/
protected function cleanFiles($files) {
foreach ($files as $file) {
if (file_exists($file)) {
try {
\OC_Helper::rmdirr($file);
} catch (\UnexpectedValueException $ex) {
$this->log->warning(
"Error deleting temporary file/folder: {file} - Reason: {error}",
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
[
'file' => $file,
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
'error' => $ex->getMessage(),
]
);
}
}
}
}
/**
* Remove old temporary files and folders that were failed to be cleaned
*/
public function cleanOld() {
$this->cleanFiles($this->getOldFiles());
}
/**
* Get all temporary files and folders generated by oc older than an hour
*
* @return string[]
*/
protected function getOldFiles() {
$cutOfTime = time() - 3600;
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
$files = [];
$dh = opendir($this->tmpBaseDir);
if ($dh) {
while (($file = readdir($dh)) !== false) {
Fix collision on temporary files + adjust permissions This changeset hardens the temporary file and directory creation to address multiple problems that may lead to exposure of files to other users, data loss or other unexpected behaviour that is impossible to debug. **[CWE-668: Exposure of Resource to Wrong Sphere](https://cwe.mitre.org/data/definitions/668.html)** The temporary file and folder handling as implemented in ownCloud is performed using a MD5 hash over `time()` concatenated with `rand()`. This is insufficiently and leads to the following security problems: The generated filename could already be used by another user. It is not verified whether the file is already used and thus temporary files might be used for another user as well resulting in all possible stuff such as "user has file of other user". Effectively this leaves us with: 1. A timestamp based on seconds (no entropy at all) 2. `rand()` which returns usually a number between 0 and 2,147,483,647 Considering the birthday paradox and that we use this method quite often (especially when handling external storage) this is quite error prone and needs to get addressed. This behaviour has been fixed by using `tempnam` instead for single temporary files. For creating temporary directories an additional postfix will be appended, the solution is for directories still not absolutely bulletproof but the best I can think about at the moment. Improvement suggestions are welcome. **[CWE-378: Creation of Temporary File With Insecure Permissions](https://cwe.mitre.org/data/definitions/378.html)** Files were created using `touch()` which defaults to a permission of 0644. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0600. **[CWE-379: Creation of Temporary File in Directory with Incorrect Permissions](https://cwe.mitre.org/data/definitions/379.html)** Files were created using `mkdir()` which defaults to a permission of 0777. Thus other users on the machine may read potentially sensitive information as `/tmp/` is world-readable. However, ownCloud always encourages users to use a dedicated machine to run the ownCloud instance and thus this is no a high severe issue. Permissions have been adjusted to 0700.Please enter the commit message for your changes.
2015-04-23 12:29:15 +00:00
if (substr($file, 0, 7) === self::TMP_PREFIX) {
$path = $this->tmpBaseDir . '/' . $file;
$mtime = filemtime($path);
if ($mtime < $cutOfTime) {
$files[] = $path;
}
}
}
}
return $files;
}
}