Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Prevent XSS on external links
  • Loading branch information
cedric-anne authored and trasher committed Apr 5, 2023
1 parent c401d00 commit 5ed5cef
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
19 changes: 16 additions & 3 deletions src/Toolbox/URL.php
Expand Up @@ -39,8 +39,9 @@ final class URL
{
/**
* Sanitize URL to prevent XSS.
* /!\ This method only prevent links on javascript scheme. To be sure that no XSS is possible, value have to be
* HTML encoded when it is printed in a HTML page.
* /!\ This method only ensure that links are corresponding to a valid URL
* (i.e. an absolute URL with a scheme or something that correspond to a path).
* To be sure that no XSS is possible, value have to be HTML encoded when it is printed in a HTML page.
*
* @param null|string $url
*
Expand All @@ -54,7 +55,19 @@ final public static function sanitizeURL(?string $url): string

$url = trim($url);

$js_pattern = '/^' . implode('\s*', str_split('javascript:')) . '/i';
$url_begin_patterns = [
// scheme followed by `//` and a hostname (absolute URL)
'[a-z]+:\/\/.+',
// `/` that corresponds to either start of a network path (e.g. `//host/path/to/file`)
// or a relative URL (e.g. `/`, `/path/to/page`, or `//anothersite.org/`)
'\/',
];
$url_pattern = '/^(' . implode('|', $url_begin_patterns) . ')/i';
if (preg_match($url_pattern, $url) !== 1) {
return '';
}

$js_pattern = '/^javascript:/i';
if (preg_match($js_pattern, $url)) {
return '';
}
Expand Down
6 changes: 4 additions & 2 deletions tests/functional/Link.php
Expand Up @@ -119,7 +119,7 @@ protected function linkContentProvider(): iterable
'link' => '[LOCATION] > [SERIAL] ([USER])',
'item' => $item,
'safe_url' => $safe_url,
'expected' => ['_location01 > ABC0004E6 (glpi)'],
'expected' => [$safe_url ? '#' : '_location01 > ABC0004E6 (glpi)'],
];

// Link that is actually a long text (it is a normal usage!)
Expand All @@ -135,7 +135,9 @@ protected function linkContentProvider(): iterable
'item' => $item,
'safe_url' => $safe_url,
'expected' => [
<<<TEXT
$safe_url
? '#'
: <<<TEXT
id: {$item->getID()}
name: Test computer
serial: ABC0004E6/X0000015
Expand Down
30 changes: 30 additions & 0 deletions tests/units/Glpi/Toolbox/URL.php
Expand Up @@ -69,16 +69,46 @@ protected function urlProvider(): iterable
'url' => 'javascript:alert(1);" title="XSS!"',
'expected' => '',
];
yield [
'url' => 'javascript:alert(1)',
'expected' => '',
];
yield [
'url' => 'javascript://%0aalert();',
'expected' => '',
];

// Invalid URL
yield [
'url' => 'ht tp://www.domain.tld/test',
'expected' => '',
];
yield [
'url' => 'http:/www.domain.tld/test',
'expected' => '',
];
yield [
'url' => '15//test',
'expected' => '',
];

// Sane URL
yield [
'url' => 'http://www.domain.tld/test',
'expected' => 'http://www.domain.tld/test',
];
yield [
'url' => '//hostname/path/to/file',
'expected' => '//hostname/path/to/file',
];
yield [
'url' => '/test?abc=12',
'expected' => '/test?abc=12',
];
yield [
'url' => '/',
'expected' => '/',
];
}

/**
Expand Down

0 comments on commit 5ed5cef

Please sign in to comment.