Problem/Motivation

While working on #2403307: RPC endpoints for user authentication: log in, check login status, log out I've detected that we can create content without permission (in this case "Access POST on Content resource").

Steps to reproduce:

1) Enable rest + hal + restui

2) Using restui module from "Settings for resource Content" - POST method, we select hal+json for "Supported formats" and cookie for "Authentication providers".

3) Go to Permissions and enable "Basic page: Create new content" for authenticated users.

4) You need to be logged in. You can log in from the default login form or using the resource from #2403307: RPC endpoints for user authentication: log in, check login status, log out :-)

5) Now add to your rest client (from the browser where you are logged in):

This path : http://mysite/entity/node

This Body :{"_links":{"type":{"href":"http://mysite/rest/type/node/page"}}, "title":[{"value":"ole!"}]}

6) Request for the token and add this token to the Header. (You also need the Accept and Content-Type headers.)

7) Send request.

8) Go to the content from admin/content and you will see the node created.

"Access POST on Content resource" permission is disabled. Looks like a security bug...

Proposed resolution

Check permissions.

CommentFileSizeAuthor
#9 2420559.patch2.38 KBklausi
#8 2420559-5.patch3.46 KBdawehner
#2 2420559.patch2.88 KBklausi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Title: Permissions are not working as expected. » REST permissions are not working as expected.
Issue tags: +D8 upgrade path

Tagging.

klausi’s picture

FileSize
2.88 KB

klausi opened a new pull request for this issue.

klausi’s picture

Here is a test case demonstrating the bug, so this should fail.

I'm not sure yet why the _permission property on the routes in ResourceBase::getBaseRoute() are not respected.

Status: Needs review » Needs work

The last submitted patch, 2: 2420559.patch, failed testing.

klausi’s picture

Status: Needs work » Needs review
FileSize
2.38 KB

klausi pushed some commits to the pull request.

For an interdiff please see the list of recent commits.

klausi’s picture

Aha, I assume this will not be the last vulnerability in the D8 life cycle caused by AccessManagerInterface::ACCESS_MODE_ANY

klausi’s picture

dawehner’s picture

FileSize
3.46 KB

I don't wanna open up that again, but just to be clear, ANY was the only option in the beginning, so we just kept that behaviour for REST. The default is ALL though now.

klausi’s picture

FileSize
2.38 KB

My patch has more comments and it passes testing at https://qa.drupal.org/pifr/test/968118 (no sure why it is yellow above).

Re-uploading patch from #5 to avoid confusion.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Yeah simply ignore my patch from above ... it just shows that the patch from klausi is similar to how other people would have tackled the problem.
I did the debugging while klausi posted the comment on the patch and I got distracted :)

dawehner’s picture

Yeah simply ignore my patch from above ... it just shows that the patch from klausi is similar to how other people would have tackled the problem.
I did the debugging while klausi posted the comment on the patch and I got distracted :)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Great. Simple fix to an obvious sechole. Committed 1eae521 and pushed to 8.0.x. Thanks!

  • alexpott committed 1eae521 on 8.0.x
    Issue #2420559 by klausi, dawehner: REST permissions are not working as...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

clemens.tolboom’s picture