Skip to content

Commit

Permalink
Fix IDN domain name not being allowed
Browse files Browse the repository at this point in the history 
The filter_var function is unfortunately not perfect and doesn't support
domain with unicode as well as url with underscores. Replace usage with
a regex from Symfony validator.

See https://bugs.php.net/search.php?cmd=display&search_for=FILTER_VALIDATE_URL

Closes #27906

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
  • Loading branch information
@CarlSchwan
CarlSchwan committed Aug 6, 2021
1 parent 30d8704 commit e62819d
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 15 deletions.
2 changes: 1 addition & 1 deletion apps/oauth2/lib/Controller/SettingsController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function __construct(string $appName,

public function addClient(string $name,
string $redirectUri): JSONResponse {
if (filter_var($redirectUri, FILTER_VALIDATE_URL) === false) {
if (!Util::isValidUrl($redirectUri)) {
return new JSONResponse(['message' => $this->l->t('Your redirect URL needs to be a full URL for example: https://yourdomain.com/path')], Http::STATUS_BAD_REQUEST);
}

Expand Down
15 changes: 4 additions & 11 deletions apps/theming/lib/Controller/ThemingController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
use OCP\IRequest;
use OCP\ITempManager;
use OCP\IURLGenerator;
use OCP\Util;

/**
* Class ThemingController
Expand Down Expand Up @@ -142,23 +143,23 @@ public function updateStylesheet($setting, $value) {
if (strlen($value) > 500) {
$error = $this->l10n->t('The given web address is too long');
}
if (!$this->isValidUrl($value)) {
if (!Util::isValidUrl($value)) {
$error = $this->l10n->t('The given web address is not a valid URL');
}
break;
case 'imprintUrl':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given legal notice address is too long');
}
if (!$this->isValidUrl($value)) {
if (!Util::isValidUrl($value)) {
$error = $this->l10n->t('The given legal notice address is not a valid URL');
}
break;
case 'privacyUrl':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given privacy policy address is too long');
}
if (!$this->isValidUrl($value)) {
if (!Util::isValidUrl($value)) {
$error = $this->l10n->t('The given privacy policy address is not a valid URL');
}
break;
Expand Down Expand Up @@ -199,14 +200,6 @@ public function updateStylesheet($setting, $value) {
);
}

/**
* Check that a string is a valid http/https url
*/
private function isValidUrl(string $url): bool {
return ((strpos($url, 'http://') === 0 || strpos($url, 'https://') === 0) &&
filter_var($url, FILTER_VALIDATE_URL) !== false);
}

/**
* @return DataResponse
* @throws NotPermittedException
Expand Down
5 changes: 3 additions & 2 deletions apps/theming/lib/ThemingDefaults.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
use OCP\IL10N;
use OCP\INavigationManager;
use OCP\IURLGenerator;
use OCP\Util as OCPUtil;

class ThemingDefaults extends \OC_Defaults {

Expand Down Expand Up @@ -157,7 +158,7 @@ public function getBaseUrl() {
}

public function getSlogan(?string $lang = null) {
return \OCP\Util::sanitizeHTML($this->config->getAppValue('theming', 'slogan', parent::getSlogan($lang)));
return OCPUtil::sanitizeHTML($this->config->getAppValue('theming', 'slogan', parent::getSlogan($lang)));
}

public function getImprintUrl() {
Expand Down Expand Up @@ -203,7 +204,7 @@ public function getShortFooter() {
$divider = '';
foreach ($links as $link) {
if ($link['url'] !== ''
&& filter_var($link['url'], FILTER_VALIDATE_URL)
&& OCPUtil::isValidUrl($link['url'])
) {
$legalLinks .= $divider . '<a href="' . $link['url'] . '" class="legal" target="_blank"' .
' rel="noreferrer noopener">' . $link['text'] . '</a>';
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Setup.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ private static function findWebRoot(SystemConfig $config): string {
if ($webRoot === '') {
throw new InvalidArgumentException('overwrite.cli.url is empty');
}
if (!filter_var($webRoot, FILTER_VALIDATE_URL)) {
if (!Util::isValidUrl($webRoot)) {
throw new InvalidArgumentException('invalid value for overwrite.cli.url');
}
$webRoot = rtrim(parse_url($webRoot, PHP_URL_PATH), '/');
Expand Down
32 changes: 32 additions & 0 deletions lib/public/Util.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,4 +513,36 @@ public static function needUpgrade() {
}
return self::$needUpgradeCache;
}

/**
* Checks whether the given URL is valid. This function should be
* used instead of filter_var($url, FILTER_VALIDATE_URL) since it
* handles idn urls too.
*
* @return bool true if the url is valid, false otherwise.
* @since 23.0.0
*/
public static function isValidUrl(string $url, array $protocols = ['http', 'https']): bool {
// from https://github.com/symfony/validator/blob/14337bdf9e2e0b2e3385c9e90f13325f0c95a4f9/Constraints/UrlValidator.php#L24
// Author: Bernhard Schussek <bschussek@gmail.com>
$pattern = '~^
(%s):// # protocol
(((?:[\_\.\pL\pN-]|%%[0-9A-Fa-f]{2})+:)?((?:[\_\.\pL\pN-]|%%[0-9A-Fa-f]{2})+)@)? # basic auth
(
([\pL\pN\pS\-\_\.])+(\.?([\pL\pN]|xn\-\-[\pL\pN-]+)+\.?) # a domain name
| # or
\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3} # an IP address
| # or
\[
(?:(?:(?:(?:(?:(?:(?:[0-9a-f]{1,4})):){6})(?:(?:(?:(?:(?:[0-9a-f]{1,4})):(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:::(?:(?:(?:[0-9a-f]{1,4})):){5})(?:(?:(?:(?:(?:[0-9a-f]{1,4})):(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:[0-9a-f]{1,4})))?::(?:(?:(?:[0-9a-f]{1,4})):){4})(?:(?:(?:(?:(?:[0-9a-f]{1,4})):(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-f]{1,4})):){0,1}(?:(?:[0-9a-f]{1,4})))?::(?:(?:(?:[0-9a-f]{1,4})):){3})(?:(?:(?:(?:(?:[0-9a-f]{1,4})):(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-f]{1,4})):){0,2}(?:(?:[0-9a-f]{1,4})))?::(?:(?:(?:[0-9a-f]{1,4})):){2})(?:(?:(?:(?:(?:[0-9a-f]{1,4})):(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-f]{1,4})):){0,3}(?:(?:[0-9a-f]{1,4})))?::(?:(?:[0-9a-f]{1,4})):)(?:(?:(?:(?:(?:[0-9a-f]{1,4})):(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-f]{1,4})):){0,4}(?:(?:[0-9a-f]{1,4})))?::)(?:(?:(?:(?:(?:[0-9a-f]{1,4})):(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9]))\.){3}(?:(?:25[0-5]|(?:[1-9]|1[0-9]|2[0-4])?[0-9])))))))|(?:(?:(?:(?:(?:(?:[0-9a-f]{1,4})):){0,5}(?:(?:[0-9a-f]{1,4})))?::)(?:(?:[0-9a-f]{1,4})))|(?:(?:(?:(?:(?:(?:[0-9a-f]{1,4})):){0,6}(?:(?:[0-9a-f]{1,4})))?::))))
\] # an IPv6 address
)
(:[0-9]+)? # a port (optional)
(?:/ (?:[\pL\pN\-._\~!$&\'()*+,;=:@]|%%[0-9A-Fa-f]{2})* )* # a path
(?:\? (?:[\pL\pN\-._\~!$&\'\[\]()*+,;=:@/?]|%%[0-9A-Fa-f]{2})* )? # a query (optional)
(?:\# (?:[\pL\pN\-._\~!$&\'()*+,;=:@/?]|%%[0-9A-Fa-f]{2})* )? # a fragment (optional)
$~ixu';
$pattern = sprintf($pattern, implode('|', $protocols));
return preg_match($pattern, $url) !== false;
}
}

0 comments on commit e62819d

Please sign in to comment.