Page MenuHomePhabricator
Files F14033561

D12169.diff
No OneTemporary
Actions

D12169.diff
View Options

diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php
--- a/src/applications/files/storage/PhabricatorFile.php
+++ b/src/applications/files/storage/PhabricatorFile.php
@@ -471,17 +471,39 @@
pht('Too many redirects trying to fetch remote URI.'));
}
- PhabricatorEnv::requireValidRemoteURIForFetch(
+ $resolved = PhabricatorEnv::requireValidRemoteURIForFetch(
$current,
array(
'http',
'https',
));
- list($status, $body, $headers) = id(new HTTPSFuture($current))
+ list($resolved_uri, $resolved_domain) = $resolved;
+
+ $current = new PhutilURI($current);
+ if ($current->getProtocol() == 'http') {
+ // For HTTP, we can use a pre-resolved URI to defuse DNS rebinding.
+ $fetch_uri = $resolved_uri;
+ $fetch_host = $resolved_domain;
+ } else {
+ // For HTTPS, we can't: cURL won't verify the SSL certificate if
+ // the domain has been replaced with an IP. But internal services
+ // presumably will not have valid certificates for rebindable
+ // domain names on attacker-controlled domains, so the DNS rebinding
+ // attack should generally not be possible anyway.
+ $fetch_uri = $current;
+ $fetch_host = null;
+ }
+
+ $future = id(new HTTPSFuture($fetch_uri))
->setFollowLocation(false)
- ->setTimeout($timeout)
- ->resolve();
+ ->setTimeout($timeout);
+
+ if ($fetch_host !== null) {
+ $future->addHeader('Host', $fetch_host);
+ }
+
+ list($status, $body, $headers) = $future->resolve();
if ($status->isRedirect()) {
// This is an HTTP 3XX status, so look for a "Location" header.
diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php
--- a/src/infrastructure/env/PhabricatorEnv.php
+++ b/src/infrastructure/env/PhabricatorEnv.php
@@ -717,7 +717,7 @@
*
* @param string URI to test.
* @param list<string> Allowed protocols.
- * @return void
+ * @return pair<string, string> Pre-resolved URI and domain.
* @task uri
*/
public static function requireValidRemoteURIForFetch(
@@ -776,6 +776,11 @@
$address));
}
}
+
+ $resolved_uri = clone $uri;
+ $resolved_uri->setDomain(head($addresses));
+
+ return array($resolved_uri, $domain);
}

File Metadata

Mime Type
text/plain
Expires
Sun, Nov 10, 6:14 PM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6715150
Default Alt Text
D12169.diff (2 KB)

Event Timeline

Phabricator · Built by Phacility