From 41270fcf9980c4be2529d2750c717675548eb617 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 24 May 2017 22:35:29 +0100 Subject: [PATCH 1/2] [SS-2017-003] Only allow HTTP(S) links for external redirector pages --- code/model/RedirectorPage.php | 24 +++++++++++++++++------- tests/model/RedirectorPageTest.php | 11 +++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/code/model/RedirectorPage.php b/code/model/RedirectorPage.php index d39c4d87..d56f1b7f 100644 --- a/code/model/RedirectorPage.php +++ b/code/model/RedirectorPage.php @@ -106,13 +106,23 @@ class RedirectorPage extends Page { public function onBeforeWrite() { parent::onBeforeWrite(); - // Prefix the URL with "http://" if no prefix is found - if( - $this->ExternalURL - && !parse_url($this->ExternalURL, PHP_URL_SCHEME) - && !preg_match('#^//#', $this->ExternalURL) - ) { - $this->ExternalURL = 'http://' . $this->ExternalURL; + if ($this->ExternalURL && substr($this->ExternalURL, 0, 2) !== '//') { + $urlParts = parse_url($this->ExternalURL); + if ($urlParts) { + if (empty($urlParts['scheme'])) { + // no scheme, assume http + $this->ExternalURL = 'http://' . $this->ExternalURL; + } elseif (!in_array($urlParts['scheme'], array( + 'http', + 'https', + ))) { + // we only allow http(s) urls + $this->ExternalURL = ''; + } + } else { + // malformed URL to reject + $this->ExternalURL = ''; + } } } diff --git a/tests/model/RedirectorPageTest.php b/tests/model/RedirectorPageTest.php index 6d4d29d7..8bdd7e1e 100644 --- a/tests/model/RedirectorPageTest.php +++ b/tests/model/RedirectorPageTest.php @@ -80,6 +80,17 @@ class RedirectorPageTest extends FunctionalTest { RedirectorPage_Controller::remove_extension('RedirectorPageTest_RedirectExtension'); } + public function testNoJSLinksAllowed() + { + $page = new RedirectorPage(); + $js = 'javascript:alert("hello world")'; + $page->ExternalURL = $js; + $this->assertEquals($js, $page->ExternalURL); + + $page->write(); + $this->assertEmpty($page->ExternalURL); + } + } class RedirectorPageTest_RedirectExtension extends Extension implements TestOnly { From 61cf72c08dafddef416d73f943ccd45e70c5d43d Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 9 May 2017 15:55:00 +0100 Subject: [PATCH 2/2] [SS-2017-004] FIX Unescaped fields in CMSPageHistroyController::compare() --- code/controllers/CMSPageHistoryController.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/code/controllers/CMSPageHistoryController.php b/code/controllers/CMSPageHistoryController.php index fa02580a..f191ed87 100644 --- a/code/controllers/CMSPageHistoryController.php +++ b/code/controllers/CMSPageHistoryController.php @@ -409,11 +409,7 @@ class CMSPageHistoryController extends CMSMain { "ID" => $id, "Version" => $fromVersion, )); - - foreach($form->Fields()->dataFields() as $field) { - $field->dontEscape = true; - } - + return $form; }