CVE-2019-14857: URL Redirection to Untrusted Site ('Open Redirect') (CWE-601)
value: A flaw was found in mod_auth_openidc before version 2.4.0.1. An open redirect issue exists in URLs with trailing slashes similar to CVE-2019-3877 in mod_auth_mellon.
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: 2019-11-26T12:15Z
Repository: https://github.com/zmartzone/mod_auth_openidc
Language: Other
Files changed: AUTHORS, ChangeLog, configure.ac, src/mod_auth_openidc.c
Commit date: 2019-10-02 21:14:15 +0200
Commit message: improve validation of the post-logout URL; closes #449

- to avoid an open redirect; thanks AIMOTO Norihito
- release 2.4.0.1

Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Diff: diff --git a/AUTHORS b/AUTHORS
index dcb9ab7..bd0becf 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -52,6 +52,8 @@ reporting bugs, providing fixes, suggesting useful features or other:
 	archzone <https://github.com/archzone>
 	Petteri Stenius <https://github.com/psteniusubi>
 	Lance Fannin <lancekf08@gmail.com>
 	Ricardo Martin Camarero <https://github.com/rickyepoderi>
 	Filip Vujicic <https://github.com/FilipVujicic>
-        Janusz Ulanowski <https://github.com/janul>
+	Janusz Ulanowski <https://github.com/janul>
+	AIMOTO Norihito
+
diff --git a/ChangeLog b/ChangeLog
index f6c103c..6c9c3c6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
+10/02/2019
+- improve validation of the post-logout URL parameter on logout; thanks AIMOTO Norihito; closes #449
+- release 2.4.0.1
+
 08/28/2019
 - fixes #447 #441 : changed storing POST params from localStorage to
   sessionStorage due to some issue of losing data in localStorage in Firefox
   (private mode) 
 
diff --git a/configure.ac b/configure.ac
index 6f77307..851f42e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
-AC_INIT([mod_auth_openidc],[2.4.0],[hans.zandbelt@zmartzone.eu])
+AC_INIT([mod_auth_openidc],[2.4.0.1],[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 3cfbb7a..7d56eef 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -3022,19 +3022,69 @@ out:
 	}
 
 	return rc;
 }
 
+static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url,
+		char **err_str, char **err_desc) {
+	apr_uri_t uri;
+	const char *c_host = NULL;
+
+	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 (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;
+	}
+
+	/* 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
  */
 static int oidc_handle_logout(request_rec *r, oidc_cfg *c,
 		oidc_session_t *session) {
 
 	oidc_provider_t *provider = NULL;
 	/* pickup the command or URL where the user wants to go after logout */
 	char *url = NULL;
+	char *error_str = NULL;
+	char *error_description = NULL;
+
 	oidc_util_get_request_parameter(r, OIDC_REDIRECT_URI_REQUEST_LOGOUT, &url);
 
 	oidc_debug(r, "enter (url=%s)", url);
 
 	if (oidc_is_front_channel_logout(url)) {
@@ -3048,48 +3098,15 @@ static int oidc_handle_logout(request_rec *r, oidc_cfg *c,
 		url = c->default_slo_url;
 
 	} else {
 
 		/* do input validation on the logout parameter value */
-
-		const char *error_description = NULL;
-		apr_uri_t uri;
-
-		if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) {
-			const char *error_description = apr_psprintf(r->pool,
-					"Logout URL malformed: %s", url);
-			oidc_error(r, "%s", error_description);
-			return oidc_util_html_send_error(r, c->error_template,
-					"Malformed URL", error_description,
-					HTTP_INTERNAL_SERVER_ERROR);
-
-		}
-
-		const char *c_host = oidc_get_current_url_host(r);
-		if ((uri.hostname != NULL)
-				&& ((strstr(c_host, uri.hostname) == NULL)
-						|| (strstr(uri.hostname, c_host) == NULL))) {
-			error_description =
-					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", error_description);
-			return oidc_util_html_send_error(r, c->error_template,
-					"Invalid Request", error_description,
-					HTTP_INTERNAL_SERVER_ERROR);
-		}
-
-		/* validate the URL to prevent HTTP header splitting */
-		if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
-			error_description =
-					apr_psprintf(r->pool,
-							"logout value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
-							url);
-			oidc_error(r, "%s", error_description);
-			return oidc_util_html_send_error(r, c->error_template,
-					"Invalid Request", error_description,
-					HTTP_INTERNAL_SERVER_ERROR);
+		if (oidc_validate_post_logout_url(r, url, &error_str,
+				&error_description) == FALSE) {
+			return oidc_util_html_send_error(r, c->error_template, error_str,
+					error_description,
+					HTTP_BAD_REQUEST);
 		}
 	}
 
 	oidc_get_provider_from_session(r, c, session, &provider);
 

Vulnerable code:         Janusz Ulanowski <https://github.com/janul>
AC_INIT([mod_auth_openidc],[2.4.0],[hans.zandbelt@zmartzone.eu])

		const char *error_description = NULL;
		apr_uri_t uri;

		if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) {
			const char *error_description = apr_psprintf(r->pool,
					"Logout URL malformed: %s", url);
			oidc_error(r, "%s", error_description);
			return oidc_util_html_send_error(r, c->error_template,
					"Malformed URL", error_description,
					HTTP_INTERNAL_SERVER_ERROR);

		}

		const char *c_host = oidc_get_current_url_host(r);
		if ((uri.hostname != NULL)
				&& ((strstr(c_host, uri.hostname) == NULL)
						|| (strstr(uri.hostname, c_host) == NULL))) {
			error_description =
					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", error_description);
			return oidc_util_html_send_error(r, c->error_template,
					"Invalid Request", error_description,
					HTTP_INTERNAL_SERVER_ERROR);
		}

		/* validate the URL to prevent HTTP header splitting */
		if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
			error_description =
					apr_psprintf(r->pool,
							"logout value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
							url);
			oidc_error(r, "%s", error_description);
			return oidc_util_html_send_error(r, c->error_template,
					"Invalid Request", error_description,
					HTTP_INTERNAL_SERVER_ERROR);
Fixed code: 	Janusz Ulanowski <https://github.com/janul>
	AIMOTO Norihito

10/02/2019
- improve validation of the post-logout URL parameter on logout; thanks AIMOTO Norihito; closes #449
- release 2.4.0.1

AC_INIT([mod_auth_openidc],[2.4.0.1],[hans.zandbelt@zmartzone.eu])
static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url,
		char **err_str, char **err_desc) {
	apr_uri_t uri;
	const char *c_host = NULL;

	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 (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;
	}

	/* 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;
}

	char *error_str = NULL;
	char *error_description = NULL;

		if (oidc_validate_post_logout_url(r, url, &error_str,
				&error_description) == FALSE) {
			return oidc_util_html_send_error(r, c->error_template, error_str,
					error_description,
					HTTP_BAD_REQUEST);