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.
Comment | File | Size | Author |
---|---|---|---|
#9 | 2420559.patch | 2.38 KB | klausi |
Comments
Comment #1
catchTagging.
Comment #2
klausiklausi opened a new pull request for this issue.
Comment #3
klausiHere 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.
Comment #5
klausiklausi pushed some commits to the pull request.
For an interdiff please see the list of recent commits.
Comment #6
klausiAha, I assume this will not be the last vulnerability in the D8 life cycle caused by AccessManagerInterface::ACCESS_MODE_ANY
Comment #7
klausiThe vulnerability was introduced in #2063401: Replace the default _access_checks(access mode) with ALL instead of ANY.
Comment #8
dawehnerI 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.
Comment #9
klausiMy 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.
Comment #10
dawehnerYeah 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 :)
Comment #11
dawehnerYeah 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 :)
Comment #12
alexpottGreat. Simple fix to an obvious sechole. Committed 1eae521 and pushed to 8.0.x. Thanks!
Comment #15
clemens.tolboomThis is reverted by #2431281: Drop support for _access_mode routes and always assume ALL