From 41270fcf9980c4be2529d2750c717675548eb617 Mon Sep 17 00:00:00 2001
From: Daniel Hensby <dhensby@silverstripe.com>
Date: Wed, 24 May 2017 22:35:29 +0100
Subject: [PATCH] [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 {