From 03e6bfb446f4e3f27c003d30d6a433e5dd8e2b3d Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Fri, 3 Sep 2021 10:41:21 +0200
Subject: [PATCH] apply OIDCRedirectURLsAllowed setting to target_link_uri

closes #672; thanks @Meheni
release 2.4.9.4

Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
---
 AUTHORS                |   2 +
 ChangeLog              |   2 +
 auth_openidc.conf      |   5 +-
 configure.ac           |   2 +-
 src/mod_auth_openidc.c | 191 ++++++++++++++++++++++-------------------
 5 files changed, 109 insertions(+), 93 deletions(-)

Index: mod_auth_openidc-2.4.0/src/mod_auth_openidc.c
===================================================================
--- mod_auth_openidc-2.4.0.orig/src/mod_auth_openidc.c
+++ mod_auth_openidc-2.4.0/src/mod_auth_openidc.c
@@ -2561,6 +2561,79 @@ static int oidc_target_link_uri_matches_
 	return TRUE;
 }
 
+#define OIDC_MAX_URL_LENGTH 8192 * 2
+
+static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c,
+		const char *redirect_to_url, apr_byte_t restrict_to_host, char **err_str,
+		char **err_desc) {
+	apr_uri_t uri;
+	const char *c_host = NULL;
+	apr_hash_index_t *hi = NULL;
+	size_t i = 0;
+	char *url = apr_pstrndup(r->pool, redirect_to_url, OIDC_MAX_URL_LENGTH);
+
+	// replace potentially harmful backslashes with forward slashes
+	for (i = 0; i < strlen(url); i++)
+		if (url[i] == '\\')
+			url[i] = '/';
+
+	if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) {
+		*err_str = apr_pstrdup(r->pool, "Malformed URL");
+		*err_desc = apr_psprintf(r->pool, "not a valid URL value: %s", url);
+		oidc_error(r, "%s: %s", *err_str, *err_desc);
+		return FALSE;
+	}
+
+	if ((uri.hostname != NULL) && (restrict_to_host == TRUE)) {
+		c_host = oidc_get_current_url_host(r);
+		if ((strstr(c_host, uri.hostname) == NULL)
+				|| (strstr(uri.hostname, c_host) == NULL)) {
+			*err_str = apr_pstrdup(r->pool, "Invalid Request");
+			*err_desc =
+					apr_psprintf(r->pool,
+							"URL value \"%s\" does not match the hostname of the current request \"%s\"",
+							apr_uri_unparse(r->pool, &uri, 0), c_host);
+			oidc_error(r, "%s: %s", *err_str, *err_desc);
+			return FALSE;
+		}
+	}
+
+	if ((uri.hostname == NULL) && (strstr(url, "/") != url)) {
+		*err_str = apr_pstrdup(r->pool, "Malformed URL");
+		*err_desc =
+				apr_psprintf(r->pool,
+						"No hostname was parsed and it does not seem to be relative, i.e starting with '/': %s",
+						url);
+		oidc_error(r, "%s: %s", *err_str, *err_desc);
+		return FALSE;
+	} else if ((uri.hostname == NULL) && (strstr(url, "//") == url)) {
+		*err_str = apr_pstrdup(r->pool, "Malformed URL");
+		*err_desc = apr_psprintf(r->pool,
+				"No hostname was parsed and starting with '//': %s", url);
+		oidc_error(r, "%s: %s", *err_str, *err_desc);
+		return FALSE;
+	} else if ((uri.hostname == NULL) && (strstr(url, "/\\") == url)) {
+		*err_str = apr_pstrdup(r->pool, "Malformed URL");
+		*err_desc = apr_psprintf(r->pool,
+				"No hostname was parsed and starting with '/\\': %s", url);
+		oidc_error(r, "%s: %s", *err_str, *err_desc);
+		return FALSE;
+	}
+
+	/* validate the URL to prevent HTTP header splitting */
+	if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
+		*err_str = apr_pstrdup(r->pool, "Invalid URL");
+		*err_desc =
+				apr_psprintf(r->pool,
+						"URL value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
+						url);
+		oidc_error(r, "%s: %s", *err_str, *err_desc);
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 /*
  * handle a response from an IDP discovery page and/or handle 3rd-party initiated SSO
  */
@@ -2571,6 +2644,8 @@ static int oidc_handle_discovery_respons
 			*auth_request_params = NULL, *csrf_cookie, *csrf_query = NULL,
 			*user = NULL, *path_scopes;
 	oidc_provider_t *provider = NULL;
+	char *error_str = NULL;
+	char *error_description = NULL;
 
 	oidc_util_get_request_parameter(r, OIDC_DISC_OP_PARAM, &issuer);
 	oidc_util_get_request_parameter(r, OIDC_DISC_USER_PARAM, &user);
@@ -2613,7 +2688,7 @@ static int oidc_handle_discovery_respons
 		target_link_uri = c->default_sso_url;
 	}
 
-	/* do open redirect prevention */
+	/* do open redirect prevention, step 1 */
 	if (oidc_target_link_uri_matches_configuration(r, c,
 			target_link_uri) == FALSE) {
 		return oidc_util_html_send_error(r, c->error_template,
@@ -2622,6 +2697,14 @@ static int oidc_handle_discovery_respons
 				HTTP_UNAUTHORIZED);
 	}
 
+	/* do input validation on the target_link_uri parameter value, step 2 */
+	if (oidc_validate_redirect_url(r, c, target_link_uri, TRUE, &error_str,
+			&error_description) == FALSE) {
+		return oidc_util_html_send_error(r, c->error_template, error_str,
+				error_description,
+				HTTP_UNAUTHORIZED);
+	}
+
 	/* see if this is a static setup */
 	if (c->metadata_dir == NULL) {
 		if ((oidc_provider_static_config(r, c, &provider) == TRUE)
@@ -3034,79 +3117,6 @@ out:
 	return rc;
 }
 
-#define OIDC_MAX_URL_LENGTH 8192 * 2
-
-static apr_byte_t oidc_validate_post_logout_url(request_rec *r,
-		const char *redirect_to_url, char **err_str, char **err_desc) {
-	apr_uri_t uri;
-	const char *c_host = NULL;
-	size_t i = 0;
-	char *url = apr_pstrndup(r->pool, redirect_to_url, OIDC_MAX_URL_LENGTH);
-
-	// replace potentially harmful backslashes with forward slashes
-	for (i = 0; i < strlen(url); i++)
-		if (url[i] == '\\')
-			url[i] = '/';
-
-	if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) {
-		*err_str = apr_pstrdup(r->pool, "Malformed URL");
-		*err_desc = apr_psprintf(r->pool, "Logout URL malformed: %s", url);
-		oidc_error(r, "%s: %s", *err_str, *err_desc);
-		return FALSE;
-	}
-
-	c_host = oidc_get_current_url_host(r);
-	if ((uri.hostname != NULL)
-			&& ((strstr(c_host, uri.hostname) == NULL)
-					|| (strstr(uri.hostname, c_host) == NULL))) {
-		*err_str = apr_pstrdup(r->pool, "Invalid Request");
-		*err_desc =
-				apr_psprintf(r->pool,
-						"logout value \"%s\" does not match the hostname of the current request \"%s\"",
-						apr_uri_unparse(r->pool, &uri, 0), c_host);
-		oidc_error(r, "%s: %s", *err_str, *err_desc);
-		return FALSE;
-	} else if ((uri.hostname == NULL) && (strstr(url, "/") != url)) {
-		*err_str = apr_pstrdup(r->pool, "Malformed URL");
-		*err_desc =
-				apr_psprintf(r->pool,
-						"No hostname was parsed and it does not seem to be relative, i.e starting with '/': %s",
-						url);
-		oidc_error(r, "%s: %s", *err_str, *err_desc);
-		return FALSE;
-        } else if ((uri.hostname == NULL) && (strstr(url, "//") == url)) {
-                *err_str = apr_pstrdup(r->pool, "Malformed URL");
-                *err_desc =
-                                apr_psprintf(r->pool,
-                                                "No hostname was parsed and starting with '//': %s",
-                                                url);
-                oidc_error(r, "%s: %s", *err_str, *err_desc);
-                return FALSE;
-        } else if ((uri.hostname == NULL) && (strstr(url, "/\\") == url)) {
-                *err_str = apr_pstrdup(r->pool, "Malformed URL");
-                *err_desc =
-                                apr_psprintf(r->pool,
-                                                "No hostname was parsed and starting with '/\\': %s",
-                                                url);
-                oidc_error(r, "%s: %s", *err_str, *err_desc);
-                return FALSE;
-	}
-
-	/* validate the URL to prevent HTTP header splitting */
-	if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
-		*err_str = apr_pstrdup(r->pool, "Invalid Request");
-		*err_desc =
-				apr_psprintf(r->pool,
-						"logout value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
-						url);
-		oidc_error(r, "%s: %s", *err_str, *err_desc);
-		return FALSE;
-	}
-
-	return TRUE;
-}
-
-
 /*
  * perform (single) logout
  */
@@ -3136,7 +3146,7 @@ static int oidc_handle_logout(request_re
 	} else {
 
 		/* do input validation on the logout parameter value */
-		if (oidc_validate_post_logout_url(r, url, &error_str,
+		if (oidc_validate_redirect_url(r, c, url, TRUE, &error_str,
 				&error_description) == FALSE) {
 			return oidc_util_html_send_error(r, c->error_template, error_str,
 					error_description,
