diff --git a/core/lib/Drupal/Component/Serialization/YamlPecl.php b/core/lib/Drupal/Component/Serialization/YamlPecl.php index ec48a29ca6d..a01f980fd78 100644 --- a/core/lib/Drupal/Component/Serialization/YamlPecl.php +++ b/core/lib/Drupal/Component/Serialization/YamlPecl.php @@ -32,6 +32,8 @@ class YamlPecl implements SerializationInterface { // Decode binary, since Symfony YAML parser encodes binary from 3.1 // onwards. ini_set('yaml.decode_binary', 1); + // We never want to unserialize !php/object. + ini_set('yaml.decode_php', 0); $init = TRUE; } // yaml_parse() will error with an empty value. diff --git a/core/lib/Drupal/Core/Test/ObjectSerialization.php b/core/lib/Drupal/Core/Test/ObjectSerialization.php new file mode 100644 index 00000000000..99d44a55c2f --- /dev/null +++ b/core/lib/Drupal/Core/Test/ObjectSerialization.php @@ -0,0 +1,24 @@ +drupalPostForm('admin/config/development/configuration/single/import', $edit, t('Import')); $this->assertRaw(t('Configuration %name depends on the %owner module that will not be installed after import.', ['%name' => 'config_test.dynamic.second', '%owner' => 'does_not_exist'])); + + // Try to preform an update which would create a PHP object if Yaml parsing + // not securely set up. + // Perform an update. + $import = << 'config_test', + 'import' => $import, + ]; + $this->drupalPostForm('admin/config/development/configuration/single/import', $edit, t('Import')); + $this->assertRaw(t('Are you sure you want to update the %name @type?', ['%name' => 'second', '@type' => 'test configuration'])); + $this->drupalPostForm(NULL, [], t('Confirm')); + $entity = $storage->load('second'); + $this->assertRaw(t('The configuration was imported successfully.')); + $this->assertTrue(is_string($entity->label()), 'Entity label is a string'); + $this->assertTrue(strpos($entity->label(), 'ObjectSerialization') > 0, 'Label contains serialized object'); } /** diff --git a/core/modules/file/file.module b/core/modules/file/file.module index a6fc3f6ebe0..6902db6812b 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -904,6 +904,14 @@ function file_save_upload($form_field_name, $validators = [], $destination = FAL // If we made it this far it's safe to record this file in the database. $file->save(); $files[$i] = $file; + // Allow an anonymous user who creates a non-public file to see it. See + // \Drupal\file\FileAccessControlHandler::checkAccess(). + if ($user->isAnonymous() && $destination_scheme !== 'public') { + $session = \Drupal::request()->getSession(); + $allowed_temp_files = $session->get('anonymous_allowed_file_ids', []); + $allowed_temp_files[$file->id()] = $file->id(); + $session->set('anonymous_allowed_file_ids', $allowed_temp_files); + } } // Add files to the cache. diff --git a/core/modules/file/src/FileAccessControlHandler.php b/core/modules/file/src/FileAccessControlHandler.php index a82c460539a..f4bda3cb7d2 100644 --- a/core/modules/file/src/FileAccessControlHandler.php +++ b/core/modules/file/src/FileAccessControlHandler.php @@ -40,8 +40,20 @@ class FileAccessControlHandler extends EntityAccessControlHandler { } elseif ($entity->getOwnerId() == $account->id()) { // This case handles new nodes, or detached files. The user who uploaded - // the file can always access if it's not yet used. - return AccessResult::allowed(); + // the file can access it even if it's not yet used. + if ($account->isAnonymous()) { + // For anonymous users, only the browser session that uploaded the + // file is positively allowed access to it. See file_save_upload(). + // @todo Implement \Drupal\Core\Entity\EntityHandlerInterface so that + // services can be more properly injected. + $allowed_fids = \Drupal::service('session')->get('anonymous_allowed_file_ids', []); + if (!empty($allowed_fids[$entity->id()])) { + return AccessResult::allowed(); + } + } + else { + return AccessResult::allowed(); + } } } @@ -79,9 +91,24 @@ class FileAccessControlHandler extends EntityAccessControlHandler { * {@inheritdoc} */ protected function checkFieldAccess($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) { - // No user can edit the status of a file. Prevents saving a new file as - // persistent before even validating it. - if ($field_definition->getName() === 'status' && $operation === 'edit') { + // Deny access to fields that should only be set on file creation, and + // "status" which should only be changed based on a file's usage. + $create_only_fields = [ + 'uri', + 'filemime', + 'filesize', + ]; + // The operation is 'edit' when the entity is being created or updated. + // Determine if the entity is being updated by checking if it is new. + $field_name = $field_definition->getName(); + if ($operation === 'edit' && $items && ($entity = $items->getEntity()) && !$entity->isNew() && in_array($field_name, $create_only_fields, TRUE)) { + return AccessResult::forbidden(); + } + // Regardless of whether the entity exists access should be denied to the + // status field as this is managed via other APIs, for example: + // - \Drupal\file\FileUsage\FileUsageBase::add() + // - \Drupal\file\Plugin\EntityReferenceSelection\FileSelection::createNewEntity() + if ($operation === 'edit' && $field_name === 'status') { return AccessResult::forbidden(); } return parent::checkFieldAccess($operation, $field_definition, $account, $items); diff --git a/core/modules/file/src/Tests/FilePrivateTest.php b/core/modules/file/src/Tests/FilePrivateTest.php index 7ecd4853940..6808a4782b7 100644 --- a/core/modules/file/src/Tests/FilePrivateTest.php +++ b/core/modules/file/src/Tests/FilePrivateTest.php @@ -6,6 +6,7 @@ use Drupal\Core\Entity\Plugin\Validation\Constraint\ReferenceAccessConstraint; use Drupal\Component\Utility\SafeMarkup; use Drupal\file\Entity\File; use Drupal\node\Entity\NodeType; +use Drupal\user\RoleInterface; /** * Uploads a test to a private node and checks access. @@ -110,6 +111,118 @@ class FilePrivateTest extends FileFieldTestBase { $this->drupalLogin($account); $this->drupalGet($file_url); $this->assertResponse(403, 'Confirmed that access is denied for another user to the temporary file.'); + + // As an anonymous user, create a temporary file with no references and + // confirm that only the session that uploaded it may view it. + $this->drupalLogout(); + user_role_change_permissions( + RoleInterface::ANONYMOUS_ID, + [ + "create $type_name content" => TRUE, + 'access content' => TRUE, + ] + ); + $test_file = $this->getTestFile('text'); + $this->drupalGet('node/add/' . $type_name); + $edit = ['files[' . $field_name . '_0]' => drupal_realpath($test_file->getFileUri())]; + $this->drupalPostForm(NULL, $edit, t('Upload')); + /** @var \Drupal\file\FileStorageInterface $file_storage */ + $file_storage = $this->container->get('entity.manager')->getStorage('file'); + $files = $file_storage->loadByProperties(['uid' => 0]); + $this->assertEqual(1, count($files), 'Loaded one anonymous file.'); + $file = end($files); + $this->assertTrue($file->isTemporary(), 'File is temporary.'); + $usage = $this->container->get('file.usage')->listUsage($file); + $this->assertFalse($usage, 'No file usage found.'); + $file_url = file_create_url($file->getFileUri()); + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that the anonymous uploader has access to the temporary file.'); + // Close the prior connection and remove the session cookie. + $this->curlClose(); + $this->curlCookies = []; + $this->cookies = []; + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that another anonymous user cannot access the temporary file.'); + + // As an anonymous user, create a permanent file, then remove all + // references to the file (so that it becomes temporary again) and confirm + // that only the session that uploaded it may view it. + $test_file = $this->getTestFile('text'); + $this->drupalGet('node/add/' . $type_name); + $edit = []; + $edit['title[0][value]'] = $this->randomMachineName(); + $edit['files[' . $field_name . '_0]'] = drupal_realpath($test_file->getFileUri()); + $this->drupalPostForm(NULL, $edit, t('Save')); + $new_node = $this->drupalGetNodeByTitle($edit['title[0][value]']); + $file_id = $new_node->{$field_name}->target_id; + $file = File::load($file_id); + $this->assertTrue($file->isPermanent(), 'File is permanent.'); + // Remove the reference to this file. + $new_node->{$field_name} = []; + $new_node->save(); + $file = File::load($file_id); + $this->assertTrue($file->isTemporary(), 'File is temporary.'); + $usage = $this->container->get('file.usage')->listUsage($file); + $this->assertFalse($usage, 'No file usage found.'); + $file_url = file_create_url($file->getFileUri()); + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that the anonymous uploader has access to the file whose references were removed.'); + // Close the prior connection and remove the session cookie. + $this->curlClose(); + $this->curlCookies = []; + $this->cookies = []; + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that another anonymous user cannot access the file whose references were removed.'); + + // As an anonymous user, create a permanent file that is referenced by a + // published node and confirm that all anonymous users may view it. + $test_file = $this->getTestFile('text'); + $this->drupalGet('node/add/' . $type_name); + $edit = []; + $edit['title[0][value]'] = $this->randomMachineName(); + $edit['files[' . $field_name . '_0]'] = drupal_realpath($test_file->getFileUri()); + $this->drupalPostForm(NULL, $edit, t('Save')); + $new_node = $this->drupalGetNodeByTitle($edit['title[0][value]']); + $file = File::load($new_node->{$field_name}->target_id); + $this->assertTrue($file->isPermanent(), 'File is permanent.'); + $usage = $this->container->get('file.usage')->listUsage($file); + $this->assertTrue($usage, 'File usage found.'); + $file_url = file_create_url($file->getFileUri()); + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that the anonymous uploader has access to the permanent file that is referenced by a published node.'); + // Close the prior connection and remove the session cookie. + $this->curlClose(); + $this->curlCookies = []; + $this->cookies = []; + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that another anonymous user also has access to the permanent file that is referenced by a published node.'); + + // As an anonymous user, create a permanent file that is referenced by an + // unpublished node and confirm that no anonymous users may view it (even + // the session that uploaded the file) because they cannot view the + // unpublished node. + $test_file = $this->getTestFile('text'); + $this->drupalGet('node/add/' . $type_name); + $edit = []; + $edit['title[0][value]'] = $this->randomMachineName(); + $edit['files[' . $field_name . '_0]'] = drupal_realpath($test_file->getFileUri()); + $this->drupalPostForm(NULL, $edit, t('Save')); + $new_node = $this->drupalGetNodeByTitle($edit['title[0][value]']); + $new_node->setPublished(FALSE); + $new_node->save(); + $file = File::load($new_node->{$field_name}->target_id); + $this->assertTrue($file->isPermanent(), 'File is permanent.'); + $usage = $this->container->get('file.usage')->listUsage($file); + $this->assertTrue($usage, 'File usage found.'); + $file_url = file_create_url($file->getFileUri()); + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that the anonymous uploader cannot access the permanent file when it is referenced by an unpublished node.'); + // Close the prior connection and remove the session cookie. + $this->curlClose(); + $this->curlCookies = []; + $this->cookies = []; + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that another anonymous user cannot access the permanent file when it is referenced by an unpublished node.'); } } diff --git a/core/modules/file/tests/src/Kernel/AccessTest.php b/core/modules/file/tests/src/Kernel/AccessTest.php index c71d7e80e75..cfd8e56838b 100644 --- a/core/modules/file/tests/src/Kernel/AccessTest.php +++ b/core/modules/file/tests/src/Kernel/AccessTest.php @@ -85,11 +85,30 @@ class AccessTest extends KernelTestBase { } /** - * Tests that the status field is not editable. + * Tests file entity field access. + * + * @see \Drupal\file\FileAccessControlHandler::checkFieldAccess() */ - public function testStatusFieldIsNotEditable() { + public function testCheckFieldAccess() { \Drupal::currentUser()->setAccount($this->user1); - $this->assertFalse($this->file->get('status')->access('edit')); + /** @var \Drupal\file\FileInterface $file */ + $file = File::create([ + 'uri' => 'public://test.png' + ]); + // While creating a file entity access will be allowed for create-only + // fields. + $this->assertTrue($file->get('uri')->access('edit')); + $this->assertTrue($file->get('filemime')->access('edit')); + $this->assertTrue($file->get('filesize')->access('edit')); + // Access to the status field is denied whilst creating a file entity. + $this->assertFalse($file->get('status')->access('edit')); + $file->save(); + // After saving the entity is no longer new and, therefore, access to + // create-only fields and the status field will be denied. + $this->assertFalse($file->get('uri')->access('edit')); + $this->assertFalse($file->get('filemime')->access('edit')); + $this->assertFalse($file->get('filesize')->access('edit')); + $this->assertFalse($file->get('status')->access('edit')); } /** diff --git a/core/modules/file/tests/src/Kernel/FileItemValidationTest.php b/core/modules/file/tests/src/Kernel/FileItemValidationTest.php index a5c2d69f72f..c9a247bf402 100644 --- a/core/modules/file/tests/src/Kernel/FileItemValidationTest.php +++ b/core/modules/file/tests/src/Kernel/FileItemValidationTest.php @@ -45,6 +45,7 @@ class FileItemValidationTest extends KernelTestBase { 'status' => 1, ]); $this->user->save(); + $this->container->get('current_user')->setAccount($this->user); } /** @@ -85,6 +86,7 @@ class FileItemValidationTest extends KernelTestBase { // Test for max filesize. $file = File::create([ 'uri' => 'vfs://drupal_root/sites/default/files/test.txt', + 'uid' => $this->user->id(), ]); $file->setPermanent(); $file->save(); diff --git a/core/modules/media/tests/src/Kernel/MediaKernelTestBase.php b/core/modules/media/tests/src/Kernel/MediaKernelTestBase.php index 5d75bed69b0..fa39962c362 100644 --- a/core/modules/media/tests/src/Kernel/MediaKernelTestBase.php +++ b/core/modules/media/tests/src/Kernel/MediaKernelTestBase.php @@ -7,6 +7,7 @@ use Drupal\KernelTests\KernelTestBase; use Drupal\media\Entity\Media; use Drupal\media\Entity\MediaType; use Drupal\media\MediaTypeInterface; +use Drupal\user\Entity\User; use org\bovigo\vfs\vfsStream; /** @@ -43,6 +44,13 @@ abstract class MediaKernelTestBase extends KernelTestBase { */ protected $testConstraintsMediaType; + /** + * A user. + * + * @var \Drupal\user\UserInterface + */ + protected $user; + /** * {@inheritdoc} */ @@ -52,6 +60,7 @@ abstract class MediaKernelTestBase extends KernelTestBase { $this->installEntitySchema('user'); $this->installEntitySchema('file'); $this->installSchema('file', 'file_usage'); + $this->installSchema('system', 'sequences'); $this->installEntitySchema('media'); $this->installConfig(['field', 'system', 'image', 'file', 'media']); @@ -59,6 +68,13 @@ abstract class MediaKernelTestBase extends KernelTestBase { $this->testMediaType = $this->createMediaType('test'); // Create a test media type with constraints. $this->testConstraintsMediaType = $this->createMediaType('test_constraints'); + + $this->user = User::create([ + 'name' => 'username', + 'status' => 1, + ]); + $this->user->save(); + $this->container->get('current_user')->setAccount($this->user); } /** @@ -117,6 +133,7 @@ abstract class MediaKernelTestBase extends KernelTestBase { $file = File::create([ 'uri' => 'vfs://drupal_root/sites/default/files/' . $filename, + 'uid' => $this->user->id(), ]); $file->setPermanent(); $file->save(); diff --git a/core/tests/Drupal/Tests/Component/Serialization/YamlPeclTest.php b/core/tests/Drupal/Tests/Component/Serialization/YamlPeclTest.php index 38840310d8f..c2e0a0d888e 100644 --- a/core/tests/Drupal/Tests/Component/Serialization/YamlPeclTest.php +++ b/core/tests/Drupal/Tests/Component/Serialization/YamlPeclTest.php @@ -26,6 +26,16 @@ class YamlPeclTest extends YamlTestBase { $this->assertEquals($data, YamlPecl::decode(YamlPecl::encode($data))); } + /** + * Ensures that php object support is disabled. + */ + public function testObjectSupportDisabled() { + $object = new \stdClass(); + $object->foo = 'bar'; + $this->assertEquals(['O:8:"stdClass":1:{s:3:"foo";s:3:"bar";}'], YamlPecl::decode(YamlPecl::encode([$object]))); + $this->assertEquals(0, ini_get('yaml.decode_php')); + } + /** * Tests decoding YAML node anchors. * diff --git a/core/tests/Drupal/Tests/Component/Serialization/YamlSymfonyTest.php b/core/tests/Drupal/Tests/Component/Serialization/YamlSymfonyTest.php index f9c2fb9d94b..86c818c18ea 100644 --- a/core/tests/Drupal/Tests/Component/Serialization/YamlSymfonyTest.php +++ b/core/tests/Drupal/Tests/Component/Serialization/YamlSymfonyTest.php @@ -63,4 +63,16 @@ class YamlSymfonyTest extends YamlTestBase { YamlSymfony::decode('foo: [ads'); } + /** + * Ensures that php object support is disabled. + * + * @covers ::encode + */ + public function testObjectSupportDisabled() { + $this->setExpectedException(InvalidDataTypeException::class, 'Object support when dumping a YAML file has been disabled.'); + $object = new \stdClass(); + $object->foo = 'bar'; + YamlSymfony::encode([$object]); + } + } diff --git a/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php b/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php index 841ac05ff48..cee0a94b12d 100644 --- a/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php +++ b/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php @@ -77,6 +77,23 @@ class YamlTest extends UnitTestCase { } } + /** + * Ensures that decoding php objects is similar for PECL and Symfony. + * + * @requires extension yaml + */ + public function testObjectSupportDisabled() { + $object = new \stdClass(); + $object->foo = 'bar'; + // In core all Yaml encoding is done via Symfony and it does not support + // objects so in order to encode an object we hace to use the PECL + // extension. + // @see \Drupal\Component\Serialization\Yaml::encode() + $yaml = YamlPecl::encode([$object]); + $this->assertEquals(['O:8:"stdClass":1:{s:3:"foo";s:3:"bar";}'], YamlPecl::decode($yaml)); + $this->assertEquals(['!php/object "O:8:\"stdClass\":1:{s:3:\"foo\";s:3:\"bar\";}"'], YamlSymfony::decode($yaml)); + } + /** * Data provider that lists all YAML files in core. */