We did not release test cases for SA-CORE-2014-006 to not reveal too much information for attackers on how to exploit the session hijacking vulnerability.

Now that more than one month has passed we can assume that most sites are updated and commit the test case.

This is only an issue for Drupal 7 since HTTPS mixed mode was removed from Drupal 8 core in #2342593: Remove mixed SSL support from core.

CommentFileSizeAuthor
#6 2399657.patch2.11 KBklausi
#3 2399657.patch2.69 KBklausi
#1 2399657.patch2.1 KBklausi

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Active » Needs review
StatusFileSize
new 2399657.patch2.1 KB

klausi opened a new pull request for this issue.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Probably needs a test-only patch that reverts the fix to make sure that it is failing when it should?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
StatusFileSize
new 2399657.patch2.69 KB

klausi opened a new pull request for this issue.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

So that is a separate pull request with a TESTONLY branch and should fail on the testbot.

Status: Needs review » Needs work

The last submitted patch, 3: 2399657.patch, failed testing.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Needs review
StatusFileSize
new 2399657.patch2.11 KB

klausi pushed some commits to the pull request.

For an interdiff please see the list of recent commits.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Perfect, testonly patch failing where we expected it.

Fixed a typo and improved comments.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Reviewed & tested by the community

Actually the test case has already received positive reviews when this was a private security issue, so pwolanin suggested that this can go directly to RTBC.

David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x - thanks!

Fixed on commit:

--- a/modules/simpletest/tests/session.test
+++ b/modules/simpletest/tests/session.test
@@ -496,7 +496,7 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
     $admin_user = $this->drupalCreateUser(array('access administration pages'));
     $standard_user = $this->drupalCreateUser(array('access content'));
 
-    // Login the admin user first on HTTP.
+    // First log in as the admin user on HTTP.
     // We cannot use $this->drupalLogin() here because we need to use the
     // special http.php URLs.
     $edit = array(

  • David_Rothstein committed 27a7201 on 7.x 
    Issue #2399657 by klausi: Add session hijacking test cases for SA-CORE-...

Status: Fixed » Closed (fixed)

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