Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pkp/pkp-lib#7575 Fixing the Open Redirect Vulnerability on user/setLocale endpoint #7578

Closed
wants to merge 2 commits into from
Closed

pkp/pkp-lib#7575 Fixing the Open Redirect Vulnerability on user/setLocale endpoint #7578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74c4f27
pkp/pkp-lib#7575 Fixing the Open Redirect Vulnerability on user/setLo…
henriqueramos Dec 26, 2021
5d15d72
pkp/pkp-lib#7575 Adding Cypress test for Locale Switcher action
henriqueramos Dec 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cypress/tests/integration/Locales.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @file cypress/tests/integration/Locales.spec.js
*
* Copyright (c) 2014-2021 Simon Fraser University
* Copyright (c) 2000-2021 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
*/

describe('Locales tests', function() {
it('Checking locale switch operation', function() {
cy.login('dbarnes', null, 'publicknowledge');
// Change locale to fr_CA
cy.get('div.app__userNav > button.pkpButton').click();
cy.get('div.pkpDropdown__content div.pkpDropdown__section > ul li:contains("Français (Canada)")').click();
cy.get('a.app__contextTitle').contains('Journal de la connaissance du public');
// Change locale to en_US
cy.get('div.app__userNav > button.pkpButton').click();
cy.get('div.pkpDropdown__content div.pkpDropdown__section > ul li:contains("English")').click();
cy.get('a.app__contextTitle').contains('Journal of Public Knowledge');
});
})
27 changes: 19 additions & 8 deletions pages/user/PKPUserHandler.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@
*/

use APP\handler\Handler;

use APP\i18n\AppLocale;
use APP\template\TemplateManager;
use PKP\config\Config;
use PKP\core\JSONMessage;

use PKP\security\Validation;
use PKP\user\InterestManager;

class PKPUserHandler extends Handler
Expand Down Expand Up @@ -50,16 +51,26 @@ public function setLocale($args, $request)
$session->setSessionVar('currentLocale', $setLocale);
}

$source = $request->getUserVar('source');
if (preg_match('#^/\w#', $source) === 1) {
$request->redirectUrl($source);
if (!isset($_SERVER['HTTP_REFERER'])) {
Copy link
Contributor

@jonasraoni jonasraoni Dec 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't watch the PR (I mean the problem vs solution), so this is just a generic comment about the HTTP_REFERER 😁

Given this header might be removed by enterprise proxies and I remember Opera (or Firefox) also had an option to disable it, I think it would be better to use the Origin header, which should be safer (if proxies attempt to break it, they will also break CORS requests), except for really old browsers.

Edit: I saw that you're using it to redirect below, so discard my comment, but I'll use this idea to create another issue.

$request->redirect(null, 'index');

return;
}

if (isset($_SERVER['HTTP_REFERER'])) {
$request->redirectUrl($_SERVER['HTTP_REFERER']);
$baseUrlParsed = parse_url(Config::getVar('general', 'base_url'));
$refererParsed = parse_url($_SERVER['HTTP_REFERER']);

if (
!isset($baseUrlParsed['host']) ||
!isset($refererParsed['host']) ||
$baseUrlParsed['host'] !== $refererParsed['host']
) {
$request->redirect(null, 'index');

return;
}

$request->redirect(null, 'index');
$request->redirectUrl($_SERVER['HTTP_REFERER']);
}

/**
Expand Down