OPENAM-8117 Fix checking of redirection URLs created internally

Author: joebandenburg

openam-server-only/src/main/webapp/oauth2c/OAuthLogout.jsp

@@ -86,7 +86,8 @@
    try {
        SSOToken ssoToken = manager.createSSOToken(request);
        String realm = ssoToken.getProperty("Organization");
-       boolean isValidGotoUrl = AuthD.getAuth().isGotoUrlValid(gotoURL, realm);
+       boolean isValidGotoUrl = AuthD.getAuth().isGotoUrlValid(gotoURL, realm)
+               && ESAPI.validator().isValidInput("URLContext", gotoURL, "HTTPURI", 2000, false);
        if (!isValidGotoUrl) {
            OAuthUtil.debugError("OAuthLogout: invalid or empty goto URL ignored on Logout page: " + gotoURL);
            gotoURL = ServiceURI + "/UI/Logout";
@@ -95,6 +96,7 @@
        OAuthUtil.debugMessage("OAuthLogout: using default goto URL because we failed to get a session");
        gotoURL = ServiceURI + "/UI/Logout";
    }
+   gotoURL = ESAPI.encoder().encodeForJavaScript(gotoURL);
    
    String logoutURL = request.getParameter(PARAM_LOGOUT_URL);
    if (logoutURL == null) {
@@ -112,6 +114,7 @@
            doYouWantToLogout = doYouWantToLogout.replace("#IDP#", OAuth2IdP);
        }
    }
+   logoutURL = ESAPI.encoder().encodeForJavaScript(logoutURL);
    String copyrightNotice = null;
    try{
        copyrightNotice = ResourceBundle.getBundle("amAuthUI", locale).getString("copyright.notice");

openam-shared/pom.xml

@@ -157,11 +157,6 @@
             <artifactId>java-ipv6</artifactId>
         </dependency>
 
-        <dependency>
-            <groupId>external</groupId>
-            <artifactId>esapiport</artifactId>
-        </dependency>
-
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>

openam-shared/src/license/THIRD-PARTY.properties

@@ -19,5 +19,4 @@
 #
 #
 #Tue Jan 12 15:44:33 GMT 2016
-external--esapiport--2013-12-04=BSD
 external--jss4--2007-08-11=MPL

openam-shared/src/main/java/org/forgerock/openam/shared/security/whitelist/RedirectUrlValidator.java

@@ -25,7 +25,6 @@
 import java.util.Collection;
 import javax.servlet.http.HttpServletRequest;
 import org.forgerock.json.JsonValue;
-import org.owasp.esapi.ESAPI;
 
 /**
  * Validates the provided redirect URL against the list of valid goto URL domains.
@@ -77,17 +76,19 @@ public boolean isRedirectUrlValid(final String url, final T configInfo) {
             DEBUG.message("Validating goto URL " + url + " against patterns:\n" + patterns);
         }
 
-        // JavaScript URIs are a common vector for XSS attacks.
-        if (url.toLowerCase().startsWith("javascript:")) {
+        if (url.length() > MAX_URL_LENGTH) {
             return false;
         }
 
         try {
             final URI uri = new URI(url);
             // Both Absolute and scheme relative URLs should be validated.
             if (!uri.isAbsolute() && !url.startsWith("//")) {
-                return ESAPI.validator().isValidInput("isRedirectUrlValid", url, "HTTPURI", MAX_URL_LENGTH,
-                        false);
+                return true;
+            }
+
+            if (uri.getScheme() != null && !uri.getScheme().equals("http") && !uri.getScheme().equals("https")) {
+                return false;
             }
         } catch (final URISyntaxException urise) {
             if (DEBUG.messageEnabled()) {
@@ -96,10 +97,6 @@ public boolean isRedirectUrlValid(final String url, final T configInfo) {
             return false;
         }
 
-        if (!ESAPI.validator().isValidInput("isRedirectUrlValid", url, "URL", MAX_URL_LENGTH, false)) {
-            return false;
-        }
-
         if (patterns == null || patterns.isEmpty()) {
             if (DEBUG.messageEnabled()) {
                 DEBUG.message("There are no patterns to validate the URL against, the goto URL is considered valid");

openam-shared/src/test/java/org/forgerock/openam/shared/security/whitelist/RedirectUrlValidatorTest.java

@@ -232,6 +232,8 @@ public void testRelativeUrlsWithoutWhitelist(String url, boolean result) {
     public Object[][] getJavaScriptCases() {
         return new Object[][]{
                 {"javascript:alert", false},
+                {"java%73cript:alert", false},
+                {"  javascript:alert", false},
                 {"JavaSCRIpt:alert", false},
                 {"/javascript:alert", true},
         };
@@ -252,15 +254,19 @@ public void testJavaScriptUrlsWithoutWhitelist(String url, boolean result) {
     @DataProvider(name = "malformed")
     public Object[][] getMalformedCases() {
         return new Object[][]{
-                {"http:abc", false},
-                {"http:/abc", false},
-                {"/a$bc", false}
+                {"/a bc", false},
+                {"/a\"", false},
+                // invalid protocol
+                {"ftp://example.com", false},
+                // This is an example of a redirect URL that OpenAM can produce
+                {"http://example.com/openam/oauth2/authorize?nonce=1234&scope=test%20test2&response_type=code&"
+                        + "client_id=myagent&redirect_uri=http%3A%2F%2Fgoogle.com", true}
         };
     }
 
     @Test(dataProvider = "malformed")
     public void testMalformedUrlsWithWhitelist(String url, boolean result) {
-        RedirectUrlValidator<String> validator = getValidator(asSet("http://example.com/*"));
+        RedirectUrlValidator<String> validator = getValidator(asSet("http://example.com/*?*"));
         assertThat(validator.isRedirectUrlValid(url, null)).isEqualTo(result);
     }