From 5f074e973c69686d93617bc97b2d67d415627858 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaime=20Pe=CC=81rez=20Crespo?= <jaime.perez@uninett.no>
Date: Tue, 10 Oct 2017 14:24:14 +0200
Subject: [PATCH] bugfix: Make sure no JS code can be injected into redirected
 URLs

In order to fix this, we first sanitize any URL given to SimpleSAML\Utils\HTTP::checkURLAllowed() so that we make sure we have a true URL without spurious characters. Secondly, we stop using an "onload" event in the body of the redirect page to trigger the redirect automatically. Instead, we use a "meta refresh" redirection.

This double remediation is because there were two issues here: one, we were printing user input inside a chunk of javascript code. The other exploits the fact that the header() function silently breaks when a null character is part of the URL given to a "Location" header. In that case, the HTTP 302 Redirection doesn't happen, and then the browser loads the HTML and goes through it, running the injected javascript.

This fixes #699.
---
 lib/SimpleSAML/Utils/HTTP.php | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/SimpleSAML/Utils/HTTP.php b/lib/SimpleSAML/Utils/HTTP.php
index a05753827..fd01223d7 100644
--- a/lib/SimpleSAML/Utils/HTTP.php
+++ b/lib/SimpleSAML/Utils/HTTP.php
@@ -186,9 +186,10 @@ class HTTP
         echo '<html xmlns="http://www.w3.org/1999/xhtml">'."\n";
         echo "  <head>\n";
         echo '    <meta http-equiv="content-type" content="text/html; charset=utf-8">'."\n";
+        echo '    <meta http-equiv="refresh" content="0;URL=\''.htmlspecialchars($url).'\'">'."\n";
         echo "    <title>Redirect</title>\n";
         echo "  </head>\n";
-        echo "  <body onload=\"window.location.replace('".htmlspecialchars($url)."');\">\n";
+        echo "  <body>\n";
         echo "    <h1>Redirect</h1>\n";
         echo '      <p>You were redirected to: <a id="redirlink" href="'.htmlspecialchars($url).'">';
         echo htmlspecialchars($url)."</a>\n";
@@ -325,6 +326,10 @@ class HTTP
         }
         $url = self::normalizeURL($url);
 
+        if (filter_var($url, FILTER_VALIDATE_URL) === false) {
+            throw new \SimpleSAML_Error_Exception('Invalid URL: '.$url);
+        }
+
         // get the white list of domains
         if ($trustedSites === null) {
             $trustedSites = \SimpleSAML_Configuration::getInstance()->getValue('trusted.url.domains', array());
-- 
GitLab