Problem/Motivation
Drupal core has a potential information disclosure or session hijacking vulnerability.
Affects 7 and 8 (and likely 6, didn't check)
The session cookies and other cookies do not have a path set on them when Drupal is installed in a subdir. Because of this, the cookies will be sent to other applications served from the same domain.
Original STR involve a cookie Drupal.visitor, but it looks like this also affects the session cookie set form PHP and possibly others.
Drupal.visitor cookies are set with "/" as the path, whereas this should be set to the actual root of the Drupal installation. If Drupal is installed in a subfolder (e.g. https://example.com/my-drupal-8), then the Drupal.visitor cookies are also accessible to the website https://example.com and other subsites.
If a module uses user_cookie_save to save data about the visitor, then this data might get disclosed outside of the current website.
function user_cookie_save(array $values) {
foreach ($values as $field => $value) {
// Set cookie for 365 days.
setrawcookie('Drupal.visitor.' . $field, rawurlencode($value), REQUEST_TIME + 31536000, '/');
}
}
Reported via the Drupal 8 security bug bounty. Approved by Drupal Security Team for public disclosure.
https://tracker.bugcrowd.com/submissions/c7ec824ef62c270a682c80a445e488f...
Reported by https://www.drupal.org/u/lva
Proposed resolution
In PHP and JS, set the site's base path as the cookie path when setting any cookie.
Remaining tasks
Postponed on #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service
decide on strategy
create patch
create tests
review
User interface changes
none
API changes
small change in cookie settings behavior
Data model changes
none
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | session_and_other-2515054-4.patch | 4.08 KB | znerol |
Comments
Comment #1
pwolanin commentedComment #2
lva commentedI reported this bug.
I can confirm that the issue also applies to session cookies.
I'm not sure if a function already exists that returns the path in which Drupal is installed. If not, the following code may be used:
Please note that the code is a bit more advanced than just stripping the "index.php" since an administrator may rename index.php to something else.
Comment #3
pwolanin commentedComment #4
znerol commentedComment #5
znerol commentedNot sure how to test
user_cookie_save(). But since there is coverage forSessionConfiguration, I believe this is okay.Nevertheless this issue should be also tested manually with a site installed in a subdirectory. The session cookie is set upon login, and a
Drupal.visitorcookie can be obtained e.g.drupal/admin/indexand toggling the descriptions.Comment #7
znerol commentedRegrettably the installer no longer logs in the user because the session cookie now has a base url relative to the installer (i.e.
/coreinstead of/).We have the following options now:
global $base_pathwhere/coreis stripped away from withinDrupalKernel::initializeRequestGlobals().corein a similar way as thehttp.php/https.phptest frontcontrollers do.Comment #8
znerol commentedFiled #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service which provides a base path also working for front controllers nested deeply in the source tree.
Comment #19
larowlanComment #20
larowlan