See:
https://www.drupal.org/SA-CORE-2015-001
http://cgit.drupalcode.org/drupal/commit/?id=b44056d2f8e8c71d35c85ec5c2f...

This issue is only for the open redirect issue involving the "destination" URL parameter and other vectors.

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

klausi, David_Rothstein, hefox, tsphethean, dstol, DamienMcKenna, Pere Orga, benjy
CommentFileSizeAuthor
#14 open-redirect-sa-2455083.14.patch25.06 KBlarowlan
#14 interdiff.txt8.87 KBlarowlan
#13 interdiff.txt934 bytesdawehner
#13 2455083-13.patch24.6 KBdawehner
#10 interdiff.txt6.04 KBdawehner
#10 2455083-10.patch24.54 KBdawehner
#8 2455083-8.patch24.4 KBdawehner
#4 open-redirect-sa-2455083.fail_.patch2.43 KBlarowlan

Comments

David_Rothstein’s picture

David_Rothstein commented
Issue summary: View changes
Related issues: +#2455079: Password reset URL access bypass fixes from SA-CORE-2015-001 need to be ported to Drupal 8
effulgentsia’s picture

effulgentsia commented
Issue tags: +D8 upgrade path

Tagging "D8 upgrade path" as well so that we don't release a supported upgrade path beta that has publicly known security exploits.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan

assigning

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review
StatusFileSize
new open-redirect-sa-2455083.fail_.patch2.43 KB

Looks like we have partial fix for this already in RedirectResponseSubscriber.
And we have a unit test already in RedirectResponseSubscriberTest.
But it doesn't cover all the cases in the SA commit.
Fail patch.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned

enough for today

Status: Needs review » Needs work

The last submitted patch, 4: open-redirect-sa-2455083.fail_.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Assigned: Unassigned » dawehner

Working on it today.

dawehner’s picture

dawehner
Primary language German
commented
Assigned: dawehner » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2455083-8.patch24.4 KB

There we go

Status: Needs review » Needs work

The last submitted patch, 8: 2455083-8.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2455083-10.patch24.54 KB
new interdiff.txt6.04 KB

Meh.

Status: Needs review » Needs work

The last submitted patch, 10: 2455083-10.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Issue tags: +Triaged D8 critical

This is a no-brainer to keep critical. Tagging.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2455083-13.patch24.6 KB
new interdiff.txt934 bytes

Let's fix it.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new interdiff.txt8.87 KB
new open-redirect-sa-2455083.14.patch25.06 KB

Fixed some minor nits found on review, I think this is ready

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed d2304f8 and pushed to 8.0.x. Thanks!

  • alexpott committed d2304f8 on 8.0.x 
    Issue #2455083 by dawehner, larowlan, klausi, David_Rothstein, hefox,...

Status: Fixed » Closed (fixed)

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