Page MenuHomePhabricator
Files F15402059

D17647.id42441.diff
No OneTemporary
Actions

D17647.id42441.diff
View Options

diff --git a/src/parser/PhutilURI.php b/src/parser/PhutilURI.php
--- a/src/parser/PhutilURI.php
+++ b/src/parser/PhutilURI.php
@@ -45,6 +45,16 @@
$type = self::TYPE_URI;
+ // Reject ambiguous URIs outright. Different versions of different clients
+ // parse these in different ways. See T12526 for discussion.
+ if (preg_match('(^[^/:]*://[^/]*[#?].*:)', $uri)) {
+ throw new Exception(
+ pht(
+ 'Rejecting ambiguous URI "%s". This URI is not formatted or '.
+ 'encoded properly.',
+ $uri));
+ }
+
$matches = null;
if (preg_match('(^([^/:]*://[^/]*)(\\?.*)\z)', $uri, $matches)) {
// If the URI is something like `idea://open?file=/path/to/file`, the
diff --git a/src/parser/__tests__/PhutilURITestCase.php b/src/parser/__tests__/PhutilURITestCase.php
--- a/src/parser/__tests__/PhutilURITestCase.php
+++ b/src/parser/__tests__/PhutilURITestCase.php
@@ -83,17 +83,38 @@
$this->assertEqual('evil.com', $uri->getDomain());
$this->assertEqual('http://u:p@evil.com?%40good.com=', (string)$uri);
- $uri = new PhutilURI('http://good.com#u:p@evil.com/');
- $this->assertEqual('good.com#u', $uri->getUser());
- $this->assertEqual('p', $uri->getPass());
- $this->assertEqual('evil.com', $uri->getDomain());
- $this->assertEqual('http://good.com%23u:p@evil.com/', (string)$uri);
-
- $uri = new PhutilURI('http://good.com?u:p@evil.com/');
- $this->assertEqual('', $uri->getUser());
- $this->assertEqual('', $uri->getPass());
- $this->assertEqual('good.com', $uri->getDomain());
- $this->assertEqual('http://good.com?u%3Ap%40evil.com%2F=', (string)$uri);
+ // The behavior of URLs in these forms differs for different versions
+ // of cURL, PHP, and other software. Because safe parsing is a tricky
+ // proposition and these URIs are almost certainly malicious, we just
+ // reject them. See T12526 for discussion.
+
+ $dangerous = array(
+ // Ambiguous encoding.
+ 'http://good.com#u:p@evil.com/' => true,
+ 'http://good.com?u:p@evil.com/' => true,
+
+ // Unambiguous encoding: with a trailing slash.
+ 'http://good.com/#u:p@evil.com/' => false,
+ 'http://good.com/?u:p@evil.com/' => false,
+
+ // Unambiguous encoding: with escaping.
+ 'http://good.com%23u:p@evil.com/' => false,
+ 'http://good.com%40u:p@evil.com/' => false,
+ );
+
+ foreach ($dangerous as $input => $expect) {
+ $caught = null;
+ try {
+ new PhutilURI($input);
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+
+ $this->assertEqual(
+ $expect,
+ ($caught instanceof $ex),
+ pht('Unexpected parse result for dangerous URI "%s".', $input));
+ }
$uri = new PhutilURI('www.example.com');
$this->assertEqual('', $uri->getProtocol());

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 8:22 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7324656
Default Alt Text
D17647.id42441.diff (2 KB)

Event Timeline

Phabricator · Built by Phacility