diff --git a/docs/simplesamlphp-changelog.md b/docs/simplesamlphp-changelog.md index f3ebbea42f01ba07b5ee1e7e7474ad35316ff3e4..9ca74cb768dfd5ab50dda92f1b098ee5b6249b9f 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 c9d8c567646859624c145f7d3fb857aa74a3475c..090af646fccad20279bc03a3d53e21532a1e24ab 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 de126d6b9106299c30c71590e90f32eb77dbf635..bc42bda49e81ffc35941429fa44d8e33b7486086 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();