From: Michael <heluecht@pirati.ca>
Date: Wed, 3 Jun 2020 05:14:45 +0000 (+0000)
Subject: Issue 8714: Make redirects more secure
X-Git-Url: https://git.mxchange.org/?a=commitdiff_plain;h=1ca804b638d50aab9812c18d05fa51a0882c4fc9;p=friendica.git

Issue 8714: Make redirects more secure
---

diff --git a/mod/redir.php b/mod/redir.php
index 56cb13a06b..cb1ca20e0e 100644
--- a/mod/redir.php
+++ b/mod/redir.php
@@ -31,6 +31,9 @@ use Friendica\Util\Network;
 use Friendica\Util\Strings;
 
 function redir_init(App $a) {
+	if (!Session::isAuthenticated()) {
+		throw new \Friendica\Network\HTTPException\ForbiddenException(DI::l10n()->t('Access denied.'));
+	}
 
 	$url = $_GET['url'] ?? '';
 	$quiet = !empty($_GET['quiet']) ? '&quiet=1' : '';
@@ -44,102 +47,105 @@ function redir_init(App $a) {
 	// Try magic auth before the legacy stuff
 	redir_magic($a, $cid, $url);
 
-	if (!empty($cid)) {
-		$fields = ['id', 'uid', 'nurl', 'url', 'addr', 'name', 'network', 'poll', 'issued-id', 'dfrn-id', 'duplex', 'pending'];
-		$contact = DBA::selectFirst('contact', $fields, ['id' => $cid, 'uid' => [0, local_user()]]);
-		if (!DBA::isResult($contact)) {
-			notice(DI::l10n()->t('Contact not found.'));
-			DI::baseUrl()->redirect();
-		}
+	if (empty($cid)) {
+		throw new \Friendica\Network\HTTPException\NotFoundException(DI::l10n()->t('Contact not found.'));
+	}
 
-		$contact_url = $contact['url'];
+	$fields = ['id', 'uid', 'nurl', 'url', 'addr', 'name', 'network', 'poll', 'issued-id', 'dfrn-id', 'duplex', 'pending'];
+	$contact = DBA::selectFirst('contact', $fields, ['id' => $cid, 'uid' => [0, local_user()]]);
+	if (!DBA::isResult($contact)) {
+		throw new \Friendica\Network\HTTPException\NotFoundException(DI::l10n()->t('Contact not found.'));
+	}
+
+	$contact_url = $contact['url'];
 
-		if (!Session::isAuthenticated() // Visitors (not logged in or not remotes) can't authenticate.
-			|| (!empty($a->contact['id']) && $a->contact['id'] == $cid)) // Local user is already authenticated.
-		{
-			$a->redirect($url ?: $contact_url);
+	if (!empty($a->contact['id']) && $a->contact['id'] == $cid) {
+		// Local user is already authenticated.
+		redir_check_url($contact_url, $url);
+		$a->redirect($url ?: $contact_url);
+	}
+
+	if ($contact['uid'] == 0 && local_user()) {
+		// Let's have a look if there is an established connection
+		// between the public contact we have found and the local user.
+		$contact = DBA::selectFirst('contact', $fields, ['nurl' => $contact['nurl'], 'uid' => local_user()]);
+
+		if (DBA::isResult($contact)) {
+			$cid = $contact['id'];
 		}
 
-		if ($contact['uid'] == 0 && local_user()) {
-			// Let's have a look if there is an established connection
-			// between the public contact we have found and the local user.
-			$contact = DBA::selectFirst('contact', $fields, ['nurl' => $contact['nurl'], 'uid' => local_user()]);
-
-			if (DBA::isResult($contact)) {
-				$cid = $contact['id'];
-			}
-
-			if (!empty($a->contact['id']) && $a->contact['id'] == $cid) {
-				// Local user is already authenticated.
-				$target_url = $url ?: $contact_url;
-				Logger::log($contact['name'] . " is already authenticated. Redirecting to " . $target_url, Logger::DEBUG);
-				$a->redirect($target_url);
-			}
+		if (!empty($a->contact['id']) && $a->contact['id'] == $cid) {
+			// Local user is already authenticated.
+			redir_check_url($contact_url, $url);
+			$target_url = $url ?: $contact_url;
+			Logger::log($contact['name'] . " is already authenticated. Redirecting to " . $target_url, Logger::DEBUG);
+			$a->redirect($target_url);
 		}
+	}
 
-		if (remote_user()) {
-			$host = substr(DI::baseUrl()->getUrlPath() . (DI::baseUrl()->getUrlPath() ? '/' . DI::baseUrl()->getUrlPath() : ''), strpos(DI::baseUrl()->getUrlPath(), '://') + 3);
-			$remotehost = substr($contact['addr'], strpos($contact['addr'], '@') + 1);
-
-			// On a local instance we have to check if the local user has already authenticated
-			// with the local contact. Otherwise the local user would ask the local contact
-			// for authentification everytime he/she is visiting a profile page of the local
-			// contact.
-			if (($host == $remotehost) && (Session::getRemoteContactID(Session::get('visitor_visiting')) == Session::get('visitor_id'))) {
-				// Remote user is already authenticated.
-				$target_url = $url ?: $contact_url;
-				Logger::log($contact['name'] . " is already authenticated. Redirecting to " . $target_url, Logger::DEBUG);
-				$a->redirect($target_url);
-			}
+	if (remote_user()) {
+		$host = substr(DI::baseUrl()->getUrlPath() . (DI::baseUrl()->getUrlPath() ? '/' . DI::baseUrl()->getUrlPath() : ''), strpos(DI::baseUrl()->getUrlPath(), '://') + 3);
+		$remotehost = substr($contact['addr'], strpos($contact['addr'], '@') + 1);
+
+		// On a local instance we have to check if the local user has already authenticated
+		// with the local contact. Otherwise the local user would ask the local contact
+		// for authentification everytime he/she is visiting a profile page of the local
+		// contact.
+		if (($host == $remotehost) && (Session::getRemoteContactID(Session::get('visitor_visiting')) == Session::get('visitor_id'))) {
+			// Remote user is already authenticated.
+			redir_check_url($contact_url, $url);
+			$target_url = $url ?: $contact_url;
+			Logger::log($contact['name'] . " is already authenticated. Redirecting to " . $target_url, Logger::DEBUG);
+			$a->redirect($target_url);
 		}
+	}
 
-		// Doing remote auth with dfrn.
-		if (local_user() && (!empty($contact['dfrn-id']) || !empty($contact['issued-id'])) && empty($contact['pending'])) {
-			$dfrn_id = $orig_id = (($contact['issued-id']) ? $contact['issued-id'] : $contact['dfrn-id']);
+	// Doing remote auth with dfrn.
+	if (local_user() && (!empty($contact['dfrn-id']) || !empty($contact['issued-id'])) && empty($contact['pending'])) {
+		$dfrn_id = $orig_id = (($contact['issued-id']) ? $contact['issued-id'] : $contact['dfrn-id']);
 
-			if ($contact['duplex'] && $contact['issued-id']) {
-				$orig_id = $contact['issued-id'];
-				$dfrn_id = '1:' . $orig_id;
-			}
-			if ($contact['duplex'] && $contact['dfrn-id']) {
-				$orig_id = $contact['dfrn-id'];
-				$dfrn_id = '0:' . $orig_id;
-			}
+		if ($contact['duplex'] && $contact['issued-id']) {
+			$orig_id = $contact['issued-id'];
+			$dfrn_id = '1:' . $orig_id;
+		}
+		if ($contact['duplex'] && $contact['dfrn-id']) {
+			$orig_id = $contact['dfrn-id'];
+			$dfrn_id = '0:' . $orig_id;
+		}
 
-			$sec = Strings::getRandomHex();
+		$sec = Strings::getRandomHex();
 
-			$fields = ['uid' => local_user(), 'cid' => $cid, 'dfrn_id' => $dfrn_id,
-				'sec' => $sec, 'expire' => time() + 45];
-			DBA::insert('profile_check', $fields);
+		$fields = ['uid' => local_user(), 'cid' => $cid, 'dfrn_id' => $dfrn_id,
+			'sec' => $sec, 'expire' => time() + 45];
+		DBA::insert('profile_check', $fields);
 
-			Logger::log('mod_redir: ' . $contact['name'] . ' ' . $sec, Logger::DEBUG);
+		Logger::log('mod_redir: ' . $contact['name'] . ' ' . $sec, Logger::DEBUG);
 
-			$dest = (!empty($url) ? '&destination_url=' . $url : '');
+		$dest = (!empty($url) ? '&destination_url=' . $url : '');
 
-			System::externalRedirect($contact['poll'] . '?dfrn_id=' . $dfrn_id
-				. '&dfrn_version=' . DFRN_PROTOCOL_VERSION . '&type=profile&sec=' . $sec . $dest . $quiet);
-		}
+		System::externalRedirect($contact['poll'] . '?dfrn_id=' . $dfrn_id
+			. '&dfrn_version=' . DFRN_PROTOCOL_VERSION . '&type=profile&sec=' . $sec . $dest . $quiet);
 
+		redir_check_url($contact_url, $url);
 		$url = $url ?: $contact_url;
 	}
 
+	if (empty($url)) {
+		throw new \Friendica\Network\HTTPException\NotFoundException(DI::l10n()->t('Contact not found.'));
+	}
+
 	// If we don't have a connected contact, redirect with
 	// the 'zrl' parameter.
-	if (!empty($url)) {
-		$my_profile = Profile::getMyURL();
+	$my_profile = Profile::getMyURL();
 
-		if (!empty($my_profile) && !Strings::compareLink($my_profile, $url)) {
-			$separator = strpos($url, '?') ? '&' : '?';
+	if (!empty($my_profile) && !Strings::compareLink($my_profile, $url)) {
+		$separator = strpos($url, '?') ? '&' : '?';
 
-			$url .= $separator . 'zrl=' . urlencode($my_profile);
-		}
-
-		Logger::log('redirecting to ' . $url, Logger::DEBUG);
-		$a->redirect($url);
+		$url .= $separator . 'zrl=' . urlencode($my_profile);
 	}
 
-	notice(DI::l10n()->t('Contact not found.'));
-	DI::baseUrl()->redirect();
+	Logger::log('redirecting to ' . $url, Logger::DEBUG);
+	$a->redirect($url);
 }
 
 function redir_magic($a, $cid, $url)
@@ -152,15 +158,10 @@ function redir_magic($a, $cid, $url)
 	$contact = DBA::selectFirst('contact', ['url'], ['id' => $cid]);
 	if (!DBA::isResult($contact)) {
 		Logger::info('Contact not found', ['id' => $cid]);
-		// Shouldn't happen under normal conditions
-		notice(DI::l10n()->t('Contact not found.'));
-		if (!empty($url)) {
-			System::externalRedirect($url);
-		} else {
-			DI::baseUrl()->redirect();
-		}
+		throw new \Friendica\Network\HTTPException\NotFoundException(DI::l10n()->t('Contact not found.'));
 	} else {
 		$contact_url = $contact['url'];
+		redir_check_url($contact_url, $url);
 		$target_url = $url ?: $contact_url;
 	}
 
@@ -184,3 +185,20 @@ function redir_magic($a, $cid, $url)
 		Logger::info('No magic for contact', ['contact' => $contact_url]);
 	}
 }
+
+function redir_check_url(string $contact_url, string $url)
+{
+	if (empty($contact_url) || empty($url)) {
+		return;
+	}
+
+	$url_host = parse_url($url, PHP_URL_HOST);
+	$contact_url_host = parse_url($contact_url, PHP_URL_HOST);
+
+	if ($url_host == $contact_url_host) {
+		return;
+	}
+
+	Logger::error('URL check host mismatch', ['contact' => $contact_url, 'url' => $url]);
+	throw new \Friendica\Network\HTTPException\ForbiddenException(DI::l10n()->t('Access denied.'));
+}
\ No newline at end of file