From 1ca804b638d50aab9812c18d05fa51a0882c4fc9 Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 3 Jun 2020 05:14:45 +0000 Subject: [PATCH 1/3] Issue 8714: Make redirects more secure --- mod/redir.php | 186 +++++++++++++++++++++++++++----------------------- 1 file changed, 102 insertions(+), 84 deletions(-) diff --git a/mod/redir.php b/mod/redir.php index 56cb13a06..cb1ca20e0 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.')); + } + + $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 (!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']; } - $contact_url = $contact['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 (!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 (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']); + + 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['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()]); + $sec = Strings::getRandomHex(); - if (DBA::isResult($contact)) { - $cid = $contact['id']; - } + $fields = ['uid' => local_user(), 'cid' => $cid, 'dfrn_id' => $dfrn_id, + 'sec' => $sec, 'expire' => time() + 45]; + DBA::insert('profile_check', $fields); - 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); - } - } + Logger::log('mod_redir: ' . $contact['name'] . ' ' . $sec, Logger::DEBUG); - 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); + $dest = (!empty($url) ? '&destination_url=' . $url : ''); - // 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); - } - } - - // 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; - } - - $sec = Strings::getRandomHex(); - - $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); - - $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 From b36fb80eb9310bec5c23df90d48a78314193ce4b Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 3 Jun 2020 09:40:43 +0000 Subject: [PATCH 2/3] File ending, Error Message --- mod/redir.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mod/redir.php b/mod/redir.php index cb1ca20e0..f38b6f7c3 100644 --- a/mod/redir.php +++ b/mod/redir.php @@ -48,7 +48,7 @@ function redir_init(App $a) { redir_magic($a, $cid, $url); if (empty($cid)) { - throw new \Friendica\Network\HTTPException\NotFoundException(DI::l10n()->t('Contact not found.')); + throw new \Friendica\Network\HTTPException\BadRequestException(DI::l10n()->t('Bad Request.')); } $fields = ['id', 'uid', 'nurl', 'url', 'addr', 'name', 'network', 'poll', 'issued-id', 'dfrn-id', 'duplex', 'pending']; @@ -201,4 +201,4 @@ function redir_check_url(string $contact_url, string $url) 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 +} From 117d0e73621b5b2062de041d176930455386ef8e Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 3 Jun 2020 10:33:54 +0000 Subject: [PATCH 3/3] Removed unused parts --- mod/redir.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mod/redir.php b/mod/redir.php index f38b6f7c3..4eb662ee3 100644 --- a/mod/redir.php +++ b/mod/redir.php @@ -125,13 +125,10 @@ function redir_init(App $a) { 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.')); + throw new \Friendica\Network\HTTPException\BadRequestException(DI::l10n()->t('Bad Request.')); } // If we don't have a connected contact, redirect with