Skip to content

Commit 27ec6cc

Browse files
committed
Fix SSRF + Information Disclosure via stylesheet links to a local network hosts
Reported by Georgios Tsimpidas (aka Frey), Security Researcher at https://i0.rs/
1 parent 10a6d1f commit 27ec6cc

File tree

7 files changed

+85
-5
lines changed

7 files changed

+85
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
- Security: Fix remote image blocking bypass via a crafted body background attribute
1111
- Security: Fix fixed position mitigation bypass via use of !important
1212
- Security: Fix XSS issue in a HTML attachment preview
13+
- Security: Fix SSRF + Information Disclosure via stylesheet links to a local network hosts
1314

1415
## Release 1.6.13
1516

composer.json-dist

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
"roundcube/rtf-html-php": "~2.1",
2121
"masterminds/html5": "~2.7.0",
2222
"bacon/bacon-qr-code": "^2.0.0",
23-
"guzzlehttp/guzzle": "^7.3.0"
23+
"guzzlehttp/guzzle": "^7.3.0",
24+
"mlocati/ip-lib": "^1.22.0"
2425
},
2526
"require-dev": {
2627
"phpunit/phpunit": "^9"

program/actions/mail/index.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ public static function washtml_link_callback($tag, $attribs, $content, $washtml)
12741274
if (isset($attrib['href'])) {
12751275
$attrib['href'] = preg_replace('/[\x00-\x1F]/', '', $attrib['href']);
12761276

1277-
if ($tag == 'link' && preg_match('/^https?:\/\//i', $attrib['href'])) {
1277+
if ($tag == 'link' && preg_match('/^https?:\/\//i', $attrib['href']) && !rcube_utils::is_local_url($attrib['href'])) {
12781278
$tempurl = 'tmp-' . md5($attrib['href']) . '.css';
12791279
$_SESSION['modcssurls'][$tempurl] = $attrib['href'];
12801280
$attrib['href'] = $rcmail->url([

program/actions/utils/modcss.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public function run($args = [])
4747
$ctype = null;
4848

4949
try {
50-
$client = rcube::get_instance()->get_http_client();
50+
$client = rcube::get_instance()->get_http_client(['allow_redirects' => false]);
5151
$response = $client->get($realurl);
5252

5353
if (!empty($response)) {

program/lib/Roundcube/rcube_utils.php

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
<?php
22

3-
/**
3+
use IPLib\Factory;
4+
5+
/*
46
+-----------------------------------------------------------------------+
57
| This file is part of the Roundcube Webmail client |
68
| |
@@ -419,6 +421,48 @@ public static function html_identifier($str, $encode = false)
419421
return asciiwords($str, true, '_');
420422
}
421423

424+
/**
425+
* Check if an URL point to a local network location.
426+
*
427+
* @param string $url
428+
*
429+
* @return bool
430+
*/
431+
public static function is_local_url($url)
432+
{
433+
$host = parse_url($url, \PHP_URL_HOST);
434+
435+
if (is_string($host)) {
436+
// TODO: This is pretty fast, but a single message can contain multiple links
437+
// to the same target, maybe we should do some in-memory caching.
438+
if ($address = Factory::parseAddressString($host = trim($host, '[]'))) {
439+
$nets = [
440+
'127.0.0.0/8', // loopback
441+
'10.0.0.0/8', // RFC1918
442+
'172.16.0.0/12', // RFC1918
443+
'192.168.0.0/16', // RFC1918
444+
'169.254.0.0/16', // link-local / cloud metadata
445+
'::1/128',
446+
'fc00::/7',
447+
];
448+
449+
foreach ($nets as $net) {
450+
$range = Factory::parseRangeString($net);
451+
if ($range->contains($address)) {
452+
return true;
453+
}
454+
}
455+
456+
return false;
457+
}
458+
459+
// FIXME: Should we accept any non-fqdn hostnames?
460+
return (bool) preg_match('/^localhost(\.localdomain)?$/i', $host);
461+
}
462+
463+
return false;
464+
}
465+
422466
/**
423467
* Replace all css definitions with #container [def]
424468
* and remove css-inlined scripting, make position style safe

program/lib/Roundcube/rcube_washtml.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ private function wash_uri($uri, $blocked_source = false, $is_image = true)
393393
}
394394

395395
if (preg_match('/^(http|https|ftp):.+/i', $uri)) {
396-
if (!empty($this->config['allow_remote'])) {
396+
if (!empty($this->config['allow_remote']) || rcube_utils::is_local_url($uri)) {
397397
return $uri;
398398
}
399399

tests/Framework/Utils.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,40 @@ function test_file2class()
556556
}
557557
}
558558

559+
/**
560+
* Test is_local_url()
561+
*
562+
* @dataProvider provide_is_local_url_cases
563+
*/
564+
#[DataProvider('provide_is_local_url_cases')]
565+
public function test_is_local_url($input, $output)
566+
{
567+
$this->assertSame($output, \rcube_utils::is_local_url($input));
568+
}
569+
570+
/**
571+
* Test-Cases for is_local_url() test
572+
*/
573+
public static function provide_is_local_url_cases(): iterable
574+
{
575+
return [
576+
// Local hosts
577+
['https://127.0.0.1', true],
578+
['https://10.1.1.1', true],
579+
['https://172.16.0.1', true],
580+
['https://192.168.0.100', true],
581+
['https://169.254.0.200', true],
582+
['http://[fc00::1]', true],
583+
['ftp://[::1]:8080', true],
584+
['//127.0.0.1', true],
585+
['http://localhost', true],
586+
['http://localhost.localdomain', true],
587+
// Non-local hosts
588+
['http://[2001:470::76:0:0:0:2]', false],
589+
['http://domain.tld', false],
590+
];
591+
}
592+
559593
/**
560594
* rcube:utils::strtotime()
561595
*/

0 commit comments

Comments
 (0)