CVE-2021-39191: URL Redirection to Untrusted Site ('Open Redirect') (CWE-601)
value: mod_auth_openidc is an authentication/authorization module for the Apache 2.x HTTP server that functions as an OpenID Connect Relying Party, authenticating users against an OpenID Connect Provider. In versions prior to 2.4.9.4, the 3rd-party init SSO functionality of mod_auth_openidc was reported to be vulnerable to an open redirect attack by supplying a crafted URL in the `target_link_uri` parameter. A patch in version 2.4.9.4 made it so that the `OIDCRedirectURLsAllowed` setting must be applied to the `target_link_uri` parameter. There are no known workarounds aside from upgrading to a patched version.
CWE description: A web application accepts a user-controlled input that specifies a link to an external site, and uses that link in a Redirect. This simplifies phishing attacks.
CVSS v3 base score: 6.1
Published: 2021-09-03T14:15Z
Repository: https://github.com/zmartzone/mod_auth_openidc
Language: Other
Files changed: AUTHORS, ChangeLog, auth_openidc.conf, configure.ac, src/mod_auth_openidc.c
Commit date: 2021-09-03 10:41:21 +0200
Commit message: 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>
Diff: diff --git a/AUTHORS b/AUTHORS
index 6cb7165c..4f77eab6 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -71,7 +71,9 @@ reporting bugs, providing fixes, suggesting useful features or other:
 	Tatsuhiko Yasumatsu <https://github.com/ty60>
 	Adam Stadler <https://github.com/tzfx>
 	Steffen Greber <https://github.com/codemaker219>
 	Iain Heggie <https://github.com/iainh>
 	Dirk Kok <https://github.com/Foxite>
+	Meheni https://github.com/Meheni
+
 
 
diff --git a/ChangeLog b/ChangeLog
index 9be02227..2445278b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,7 +1,9 @@
 09/03/2021
 - don't apply authz in discovery process; fixes 2.4.9.3
+- apply OIDCRedirectURLsAllowed setting to target_link_uri; closes #672; thanks @Meheni
+- release 2.4.9.4
 
 08/26/2021
 - don't apply authz to the redirect URI; fixes ac5686495a51bc93e257e42bfdc9c9c46252feb1
 - bump to 2.4.9.3
 
diff --git a/auth_openidc.conf b/auth_openidc.conf
index e5352c0f..34b8a9c2 100644
--- a/auth_openidc.conf
+++ b/auth_openidc.conf
@@ -916,12 +916,13 @@
 # for calculating the fingerprint of the state during authentication.
 # When not defined the default "both" is used.
 #OIDCStateInputHeaders [none|user-agent|x-forwarded-for|both]
 
 # Define one or more regular expressions that specify URLs (or domains) allowed for post logout and
-# other redirects such as the "return_to" value on refresh token requests, and the "login_uri" value
-# on session management based logins through the OP iframe, e.g.:
+# other redirects such as the "return_to" value on refresh token requests, the "login_uri" value
+# on session management based logins through the OP iframe, and the "target_link_uri" parameter in
+# 3rd-party initiated logins, e.g.:
 #   OIDCRedirectURLsAllowed ^https://www.example.com ^https://(\w+).example.org ^https://example.net/app
 # or:
 #   OIDCRedirectURLsAllowed ^https://www.example.com/logout$ ^https://www.example.com/app/return_to$ 
 # When not defined, the default is to match the hostname in the URL redirected to against
 # the hostname in the current request.
diff --git a/configure.ac b/configure.ac
index ef3b864e..321fb33c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
-AC_INIT([mod_auth_openidc],[2.4.9.3],[hans.zandbelt@zmartzone.eu])
+AC_INIT([mod_auth_openidc],[2.4.9.4],[hans.zandbelt@zmartzone.eu])
 
 AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())
 
 # This section defines the --with-apxs2 option.
 AC_ARG_WITH(
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index 90f8b214..b36f6c1a 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -2455,20 +2455,112 @@ static int oidc_target_link_uri_matches_configuration(request_rec *r,
 		}
 	}
 	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 (c->redirect_urls_allowed != NULL) {
+		for (hi = apr_hash_first(NULL, c->redirect_urls_allowed); hi; hi =
+				apr_hash_next(hi)) {
+			apr_hash_this(hi, (const void**) &c_host, NULL, NULL);
+			if (oidc_util_regexp_first_match(r->pool, url, c_host,
+					NULL, err_str) == TRUE)
+				break;
+		}
+		if (hi == NULL) {
+			*err_str = apr_pstrdup(r->pool, "URL not allowed");
+			*err_desc =
+					apr_psprintf(r->pool,
+							"value does not match the list of allowed redirect URLs: %s",
+							url);
+			oidc_error(r, "%s: %s", *err_str, *err_desc);
+			return FALSE;
+		}
+	} else 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
  */
 static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) {
 
 	/* variables to hold the values returned in the response */
 	char *issuer = NULL, *target_link_uri = NULL, *login_hint = NULL,
 			*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);
 	oidc_util_get_request_parameter(r, OIDC_DISC_RT_PARAM, &target_link_uri);
 	oidc_util_get_request_parameter(r, OIDC_DISC_LH_PARAM, &login_hint);
@@ -2508,19 +2600,27 @@ static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) {
 					HTTP_INTERNAL_SERVER_ERROR);
 		}
 		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,
 				"Invalid Request",
 				"\"target_link_uri\" parameter does not match configuration settings, aborting to prevent an open redirect.",
 				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)
 				&& (issuer != NULL)) {
 			if (apr_strnatcmp(provider->issuer, issuer) != 0) {
@@ -2945,99 +3045,10 @@ out:
 	oidc_util_hdr_err_out_add(r, OIDC_HTTP_HDR_PRAGMA, "no-cache");
 
 	return rc;
 }
 
-#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 (c->redirect_urls_allowed != NULL) {
-		for (hi = apr_hash_first(NULL, c->redirect_urls_allowed); hi; hi =
-				apr_hash_next(hi)) {
-			apr_hash_this(hi, (const void**) &c_host, NULL, NULL);
-			if (oidc_util_regexp_first_match(r->pool, url, c_host,
-					NULL, err_str) == TRUE)
-				break;
-		}
-		if (hi == NULL) {
-			*err_str = apr_pstrdup(r->pool, "URL not allowed");
-			*err_desc =
-					apr_psprintf(r->pool,
-							"value does not match the list of allowed redirect URLs: %s",
-							url);
-			oidc_error(r, "%s: %s", *err_str, *err_desc);
-			return FALSE;
-		}
-	} else 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;
-}
 
 /*
  * perform (single) logout
  */
 static int oidc_handle_logout(request_rec *r, oidc_cfg *c,

Vulnerable code: # other redirects such as the "return_to" value on refresh token requests, and the "login_uri" value
# on session management based logins through the OP iframe, e.g.:
AC_INIT([mod_auth_openidc],[2.4.9.3],[hans.zandbelt@zmartzone.eu])
	/* do open redirect prevention */
#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 (c->redirect_urls_allowed != NULL) {
		for (hi = apr_hash_first(NULL, c->redirect_urls_allowed); hi; hi =
				apr_hash_next(hi)) {
			apr_hash_this(hi, (const void**) &c_host, NULL, NULL);
			if (oidc_util_regexp_first_match(r->pool, url, c_host,
					NULL, err_str) == TRUE)
				break;
		}
		if (hi == NULL) {
			*err_str = apr_pstrdup(r->pool, "URL not allowed");
			*err_desc =
					apr_psprintf(r->pool,
							"value does not match the list of allowed redirect URLs: %s",
							url);
			oidc_error(r, "%s: %s", *err_str, *err_desc);
			return FALSE;
		}
	} else 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;
}
Fixed code: 	Meheni https://github.com/Meheni

- apply OIDCRedirectURLsAllowed setting to target_link_uri; closes #672; thanks @Meheni
- release 2.4.9.4
# other redirects such as the "return_to" value on refresh token requests, the "login_uri" value
# on session management based logins through the OP iframe, and the "target_link_uri" parameter in
# 3rd-party initiated logins, e.g.:
AC_INIT([mod_auth_openidc],[2.4.9.4],[hans.zandbelt@zmartzone.eu])
#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 (c->redirect_urls_allowed != NULL) {
		for (hi = apr_hash_first(NULL, c->redirect_urls_allowed); hi; hi =
				apr_hash_next(hi)) {
			apr_hash_this(hi, (const void**) &c_host, NULL, NULL);
			if (oidc_util_regexp_first_match(r->pool, url, c_host,
					NULL, err_str) == TRUE)
				break;
		}
		if (hi == NULL) {
			*err_str = apr_pstrdup(r->pool, "URL not allowed");
			*err_desc =
					apr_psprintf(r->pool,
							"value does not match the list of allowed redirect URLs: %s",
							url);
			oidc_error(r, "%s: %s", *err_str, *err_desc);
			return FALSE;
		}
	} else 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;
}

	char *error_str = NULL;
	char *error_description = NULL;
	/* do open redirect prevention, step 1 */
	/* 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);
	}