From 86af04c1c9cd48b07c31a7206882959bdb3e04de Mon Sep 17 00:00:00 2001
From: "Aaron St. Clair" <gfxdude2010@gmail.com>
Date: Tue, 14 Apr 2020 10:16:54 -0400
Subject: [PATCH] Issue #1272 - SSP refuses to use temp dir if it doesn't own
it, even though it can write to it (#1314)
* Issue #1272 - SSP refuses to use temp dir if it doesn't own it, even though it can write to it
This has been addressed using the is_writable function instead of checking for UID, which only works in a Linux environment
Co-authored-by: Aaron St. Clair <astclair@ecrs.com>
---
docs/simplesamlphp-changelog.md | 1 +
lib/SimpleSAML/Utils/System.php | 23 +++++++++++++----------
tests/lib/SimpleSAML/Utils/SystemTest.php | 11 ++---------
3 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/docs/simplesamlphp-changelog.md b/docs/simplesamlphp-changelog.md
index f3ebbea42..9ca74cb76 100644
--- a/docs/simplesamlphp-changelog.md
+++ b/docs/simplesamlphp-changelog.md
@@ -16,6 +16,7 @@ Released TBD
* Raised minimum PHP version to 7.1
* Dropped support for Symfony 3.x
* Update the SAML2 library dependency to 4.1.9
+ * Fix a bug where SSP wouldn't write to the tmp-directory if it didn't own it, but could write to it (#1314)
* Allow additional audiences to be specified (#1345)
* Support saml:Extensions in saml:SP authsources (#1349)
* The `attributename`-setting in the core:TargetedID authproc-filter has been deprecated in
diff --git a/lib/SimpleSAML/Utils/System.php b/lib/SimpleSAML/Utils/System.php
index c9d8c5676..090af646f 100644
--- a/lib/SimpleSAML/Utils/System.php
+++ b/lib/SimpleSAML/Utils/System.php
@@ -66,12 +66,13 @@ class System
* This function retrieves the path to a directory where temporary files can be saved.
*
* @return string Path to a temporary directory, without a trailing directory separator.
- * @throws Error\Exception If the temporary directory cannot be created or it exists and does not belong
- * to the current user.
+ * @throws Error\Exception If the temporary directory cannot be created or it exists and cannot be written
+ * to by the current user.
*
* @author Andreas Solberg, UNINETT AS <andreas.solberg@uninett.no>
* @author Olav Morken, UNINETT AS <olav.morken@uninett.no>
* @author Jaime Perez, UNINETT AS <jaime.perez@uninett.no>
+ * @author Aaron St. Clair, ECRS AS <astclair@ecrs.com>
*/
public static function getTempDir()
{
@@ -85,6 +86,10 @@ class System
DIRECTORY_SEPARATOR
);
+ /**
+ * If the temporary directory does not exist then attempt to create it. If the temporary directory
+ * already exists then verify the current user can write to it. Otherwise, throw an error.
+ */
if (!is_dir($tempDir)) {
if (!mkdir($tempDir, 0700, true)) {
$error = error_get_last();
@@ -93,14 +98,12 @@ class System
(is_array($error) ? $error['message'] : 'no error available')
);
}
- } elseif (function_exists('posix_getuid')) {
- // check that the owner of the temp directory is the current user
- $stat = lstat($tempDir);
- if ($stat['uid'] !== posix_getuid()) {
- throw new Error\Exception(
- 'Temporary directory "' . $tempDir . '" does not belong to the current user.'
- );
- }
+ } elseif (!is_writable($tempDir)) {
+ throw new Error\Exception(
+ 'Temporary directory "' . $tempDir .
+ '" cannot be written to by the current user' .
+ (function_exists('posix_getuid') ? ' "' . posix_getuid() . '"' : '')
+ );
}
return $tempDir;
diff --git a/tests/lib/SimpleSAML/Utils/SystemTest.php b/tests/lib/SimpleSAML/Utils/SystemTest.php
index de126d6b9..bc42bda49 100644
--- a/tests/lib/SimpleSAML/Utils/SystemTest.php
+++ b/tests/lib/SimpleSAML/Utils/SystemTest.php
@@ -282,23 +282,16 @@ class SystemTest extends TestCase
/**
- * @requires OS Linux
* @covers \SimpleSAML\Utils\System::getTempDir
* @test
* @return void
*/
- public function testGetTempDirBadOwner()
+ public function testGetTempDirBadPermissions()
{
- if (!function_exists('posix_getuid')) {
- static::markTestSkipped('POSIX-functions not available; skipping!');
- }
-
- $bad_uid = posix_getuid() + 1;
-
$tempdir = $this->root_directory . DIRECTORY_SEPARATOR . self::DEFAULTTEMPDIR;
$config = $this->setConfigurationTempDir($tempdir);
- chown($tempdir, $bad_uid);
+ chmod($tempdir, 0440);
$this->expectException(\SimpleSAML\Error\Exception::class);
System::getTempDir();
--
GitLab