From 4ecfbc393d27cfe14e4b4698d3ca85ff49607e2b Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Fri, 14 Feb 2025 15:18:26 +0800 Subject: [PATCH] MDL-83762 core_files: Bind resolve IPs and ports to cURL calls --- lib/classes/files/curl_security_helper.php | 53 +++++++++++++++++++++- lib/filelib.php | 13 ++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/lib/classes/files/curl_security_helper.php b/lib/classes/files/curl_security_helper.php index 26e07bab6ae..0256dfc4ecf 100644 --- a/lib/classes/files/curl_security_helper.php +++ b/lib/classes/files/curl_security_helper.php @@ -54,6 +54,21 @@ class curl_security_helper extends curl_security_helper_base { 'https' => 443 ]; + /** + * @var string the host of the URL being checked by the helper. + */ + protected $host; + + /** + * @var array IP address or addresses the URL is allowed to be requested from (passed the blocked hosts check). + */ + protected $allowedips = []; + + /** + * @var ?int The port the URL is allowed to be requested from (passed the allowed port check). + */ + protected $allowedport; + /** * Checks whether the given URL is blocked by checking its address and port number against the allow/block lists. * The behaviour of this function can be classified as strict, as it returns true for URLs which are invalid or @@ -85,6 +100,8 @@ class curl_security_helper extends curl_security_helper_base { return true; } + $this->host = $parsed['host']; + // The port will be empty unless explicitly set in the $url (uncommon), so try to infer it from the supported schemes. if (!$parsed['port'] && $parsed['scheme'] && isset($this->transportschemes[$parsed['scheme']])) { $parsed['port'] = $this->transportschemes[$parsed['scheme']]; @@ -162,12 +179,17 @@ class curl_security_helper extends curl_security_helper_base { return true; } - // If any of the returned IPs are in the blocklist, block the request. + // If any of the returned IPs are in the blocklist, block the request. Otherwise, temporarily record the IPs. + $allowedips = []; foreach ($hostips as $hostip) { if ($this->address_explicitly_blocked($hostip)) { return true; } + $allowedips[] = $hostip; } + + // If none of the IPs are blocked, set them on the allow list so we can enforce them on subsequent requests. + $this->allowedips = $allowedips; } } else { // Was not something we consider to be a valid IP or domain name, block it. @@ -201,7 +223,15 @@ class curl_security_helper extends curl_security_helper_base { return true; } $allowedports = $this->get_allowed_ports(); - return !empty($allowedports) && !in_array($portnum, $allowedports); + + $isblocked = !empty($allowedports) && !in_array($portnum, $allowedports); + + // If port is allowed, add it to our allow list so we can enforce it on subsequent requests. + if (!$isblocked) { + $this->allowedport = $portnum; + } + + return $isblocked; } /** @@ -291,4 +321,23 @@ class curl_security_helper extends curl_security_helper_base { return !empty($entry); }); } + + /** + * Helper that returns host, IP and port information for the URL that has passed the blocked hosts/allowed ports checks. + * + * This data is in a format compatible with CURLOPT_RESOLVE, so it can be passed directly into that option. + * Doing so will prevent cURL re-fetching the info from DNS, preventing subsequent requests to the remote host from + * modifying the IP/port to ones that haven't been validated. + * + * @return array of strings in the format hostname:port:ip_address. + * @throws \coding_exception + */ + public function get_resolve_info(): array { + if (empty($this->host || empty($this->allowedips) || empty($this->allowedport))) { + $exception = 'In the curl_security_helper class, url_is_blocked() must be called before get_resolve_info() is called.'; + throw new \core\exception\coding_exception($exception); + } + + return array_map(fn($ip) => "$this->host:$this->allowedport:$ip", $this->allowedips); + } } diff --git a/lib/filelib.php b/lib/filelib.php index d8161d0461b..a5d62cc0aeb 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -3169,6 +3169,8 @@ class curl { private $ignoresecurity; /** @var array $mockresponses For unit testing only - return the head of this list instead of making the next request. */ private static $mockresponses = []; + /** @var array $curlresolveinfo Resolve addresses for the URL that have passed cuRL security checks, in a CURLOPT_RESOLVE compatible format. */ + private $curlresolveinfo = []; /** @var array temporary params value if the value is not belongs to class stored_file. */ public $_tmp_file_post_params = []; @@ -3766,6 +3768,9 @@ class curl { return $this->error; } + // Set allowed resolve info if the URL is not blocked. + $this->curlresolveinfo = $this->securityhelper->get_resolve_info(); + return null; } @@ -3802,6 +3807,10 @@ class curl { // Set the URL as a curl option. $this->setopt(array('CURLOPT_URL' => $url)); + // Force cURL to only resolve the URL from IP/port combinations that were validated by the security helper. + // This prevents re-fetching DNS data on subsequent requests, which could return un-validated hosts/ports. + $this->setopt(['CURLOPT_RESOLVE' => $this->curlresolveinfo]); + // Create curl instance. $curl = curl_init(); @@ -3913,6 +3922,10 @@ class curl { curl_setopt($curl, CURLOPT_URL, $redirecturl); + // Force cURL to only resolve the URL from IP/port combinations that were validated by the security helper. + // This prevents re-fetching DNS data on subsequent requests, which could return un-validated hosts/ports. + $this->setopt(['CURLOPT_RESOLVE' => $this->curlresolveinfo]); + // If CURLOPT_UNRESTRICTED_AUTH is empty/false, don't send credentials to other hosts. // Ref: https://curl.se/libcurl/c/CURLOPT_UNRESTRICTED_AUTH.html. $isdifferenthost = parse_url($currenturl)['host'] !== parse_url($redirecturl)['host'];