See: https://www.drupal.org/SA-CORE-2015-003
http://cgit.drupalcode.org/drupal/commit/?h=7.x&id=731dfacab8bf39918c135...

Users without the "access content" permission can see the titles of nodes that they do not have access to, if the nodes are added to a menu on the site that the users have access to.

This issue needs verifying against Drupal 8, since so much of the menu system was changed.

Credit for the D6/D7 version of this patch (the security release):

David_Rothstein, matt2000, scor, greggles, meichr, larowlan
CommentFileSizeAuthor
#12 tests_for_information-2554239-12.patch2.19 KBstrykaizer
#10 tests_for_information-2554239-10.patch1.88 KBstrykaizer
#9 tests_for_information-2554239-9.patch1.84 KBstrykaizer
#5 Screenshot 2015-08-20 16.52.25.png151.46 KBlarowlan
#3 Screen Shot 2015-08-19 at 8.11.43 PM.png97.63 KBwebchick

Comments

webchick created an issue. See original summary.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Assigned: Unassigned » webchick

Going to attempt to verify this one.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new Screen Shot 2015-08-19 at 8.11.43 PM.png97.63 KB

Nope, this isn't an issue in Drupal 8.

Steps to reproduce (/via David_Rothstein):

1. Create a node and add it to the main menu.
2. Remove the "access content" permission from anonymous users.
3. Log out and view the home page.

Works as expected.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Menu link access denied for anon.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue summary: View changes
Status: Active » Closed (cannot reproduce)
StatusFileSize
new Screenshot 2015-08-20 16.52.25.png151.46 KB

Confirming cannot reproduce in D8

dawehner’s picture

dawehner
Primary language German
commented

Is it just me or should we maybe have some dedicated test coverage for it? I mean its like a regression we should absolute never have again.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Port Information Disclosure in Menu Links - Access system fixes from SA-CORE-2015-003 to Drupal 8 » Tests for Information Disclosure in Menu Links - Access system fixes from SA-CORE-2015-003 to Drupal 8
Assigned: webchick » Unassigned
Priority: Critical » Major
Status: Closed (cannot reproduce) » Active
Issue tags: +Needs backport to D7, +Needs tests

That's true. We could totally do that.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Tests for Information Disclosure in Menu Links - Access system fixes from SA-CORE-2015-003 to Drupal 8 » Tests for Information Disclosure in Menu Links - Access system fixes from SA-CORE-2015-003
strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
Status: Active » Needs review
StatusFileSize
new tests_for_information-2554239-9.patch1.84 KB

Here's a test for D8.

As noted, this test does not fail, since this bug is not an issue at this moment in d8.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
StatusFileSize
new tests_for_information-2554239-10.patch1.88 KB

Documentation fixes

stefan.r’s picture

stefan.r commented
Status: Needs review » Needs work
Issue tags: -Needs tests 
  1. +++ b/core/modules/node/src/Tests/NodeAccessMenuLinkTest.php
@@ -0,0 +1,69 @@
+namespace Drupal\node\Tests;
+use Drupal\user\RoleInterface;

    Nit: needs a newline between these. 

  2. +++ b/core/modules/node/src/Tests/NodeAccessMenuLinkTest.php
@@ -0,0 +1,69 @@
+ * Tests the interaction of the node access system with fields.

    s/fields/menu links/ 

  3. +++ b/core/modules/node/src/Tests/NodeAccessMenuLinkTest.php
@@ -0,0 +1,69 @@
+  public static $modules = array('menu_ui', 'block');
+++ b/core/modules/node/src/Tests/NodeAccessMenuLinkTest.php
@@ -0,0 +1,69 @@
+    $edit = array(

    Nit: Bracket notation. 

  4. +++ b/core/modules/node/src/Tests/NodeAccessMenuLinkTest.php
@@ -0,0 +1,69 @@
+    // this menu link

    Missing full stop. 

  5. +++ b/core/modules/node/src/Tests/NodeAccessMenuLinkTest.php
@@ -0,0 +1,69 @@
+    $this->assertNoLink($menu_link_title);

    Just to isolate this test to the "access content" permission, can we give just that permission to the anonymous user and assert that the link is there? 

  6. +++ b/core/modules/node/src/Tests/NodeAccessMenuLinkTest.php
@@ -0,0 +1,69 @@
+  }

    Nit: missing newline before class closing bracket.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
Status: Needs work » Needs review
StatusFileSize
new tests_for_information-2554239-12.patch2.19 KB

@stefan.r: Thanks for the review!

Attached you'll find a revised patch

stefan.r’s picture

stefan.r commented

This looks great now as it tests for the same D7 vulnerability!

Just a nit: you missed converting the 4 other array()s and the first menu link assertion out of the 3 isn't necessary.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Looks fine. It'd be great if we can mention https://www.drupal.org/SA-CORE-2015-003 in someplace inside the test.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Grepped around a bit, and the format seems to be:

// DRUPAL-SA-XXXX-YYY: Blah de blah de blah.

...right above the test.

So added this:

+   * SA-CORE-2015-003: Tests menu links to nodes when node access is restricted.

(We seem to have dropped "DRUPAL" from the SAs sometime.)

...and committed/pushed to 8.0.x. Thanks!

  • webchick committed 4ebc668 on 8.0.x 
    Issue #2554239 by StryKaizer, stefan.r, David_Rothstein, matt2000, scor...
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Thank you @webchick for fixing that on commit. :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 8.0.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)

Oops, meant to do this.

  • webchick committed 4ebc668 on 8.1.x 
    Issue #2554239 by StryKaizer, stefan.r, David_Rothstein, matt2000, scor...

  • webchick committed 4ebc668 on 8.3.x 
    Issue #2554239 by StryKaizer, stefan.r, David_Rothstein, matt2000, scor...

  • webchick committed 4ebc668 on 8.3.x 
    Issue #2554239 by StryKaizer, stefan.r, David_Rothstein, matt2000, scor...

  • webchick committed 4ebc668 on 8.4.x 
    Issue #2554239 by StryKaizer, stefan.r, David_Rothstein, matt2000, scor...

  • webchick committed 4ebc668 on 8.4.x 
    Issue #2554239 by StryKaizer, stefan.r, David_Rothstein, matt2000, scor...

Status: Patch (to be ported) » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.