diff --git a/lib/filelib.php b/lib/filelib.php index a64582be23f..b8e9adea76c 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -3079,7 +3079,7 @@ class curl { public $error; /** @var int error code */ public $errno; - /** @var bool use workaround for open_basedir restrictions, to be changed from unit tests only! */ + /** @var bool Perform redirects at PHP level instead of relying on native cURL functionality. Always true now. */ public $emulateredirects = null; /** @var array cURL options */ @@ -3176,9 +3176,13 @@ class curl { $this->proxy = false; } - if (!isset($this->emulateredirects)) { - $this->emulateredirects = ini_get('open_basedir'); - } + // All redirects are performed at PHP level now and each one is checked against blocked URLs rules. We do not + // want to let cURL naively follow the redirect chain and visit every URL for security reasons. Even when the + // caller explicitly wants to ignore the security checks, we would need to fall back to the original + // implementation and use emulated redirects if open_basedir is in effect to avoid the PHP warning + // "CURLOPT_FOLLOWLOCATION cannot be activated when in safe_mode or an open_basedir". So it is better to simply + // ignore this property and always handle redirects at this PHP wrapper level and not inside the native cURL. + $this->emulateredirects = true; // Curl security setup. Allow injection of a security helper, but if not found, default to the core helper. if (isset($settings['securityhelper']) && $settings['securityhelper'] instanceof \core\files\curl_security_helper_base) { @@ -3487,8 +3491,8 @@ class curl { // Set options. foreach($this->options as $name => $val) { - if ($name === 'CURLOPT_FOLLOWLOCATION' and $this->emulateredirects) { - // The redirects are emulated elsewhere. + if ($name === 'CURLOPT_FOLLOWLOCATION') { + // All the redirects are emulated at PHP level. curl_setopt($curl, CURLOPT_FOLLOWLOCATION, 0); continue; } @@ -3705,7 +3709,11 @@ class curl { } } - // This will only check the base url. In the case of redirects, the blocking check is also after the curl_exec. + if (empty($this->emulateredirects)) { + // Just in case someone had tried to explicitly disable emulated redirects in legacy code. + debugging('Attempting to disable emulated redirects has no effect any more!', DEBUG_DEVELOPER); + } + $urlisblocked = $this->check_securityhelper_blocklist($url); if (!is_null($urlisblocked)) { return $urlisblocked; @@ -3728,17 +3736,14 @@ class curl { $this->errno = curl_errno($curl); // Note: $this->response and $this->rawresponse are filled by $hits->formatHeader callback. - // In the case of redirects (which curl blindly follows), check the post-redirect URL against the list of blocked list too. if (intval($this->info['redirect_count']) > 0) { - $urlisblocked = $this->check_securityhelper_blocklist($this->info['url']); - if (!is_null($urlisblocked)) { - $this->reset_request_state_vars(); - curl_close($curl); - return $urlisblocked; - } + // For security reasons we do not allow the cURL handle to follow redirects on its own. + // See setting CURLOPT_FOLLOWLOCATION in {@see self::apply_opt()} method. + throw new coding_exception('Internal cURL handle should never follow redirects on its own!', + 'Reported number of redirects: ' . $this->info['redirect_count']); } - if ($this->emulateredirects and $this->options['CURLOPT_FOLLOWLOCATION'] and $this->info['http_code'] != 200) { + if ($this->options['CURLOPT_FOLLOWLOCATION'] && $this->info['http_code'] != 200) { $redirects = 0; while($redirects <= $this->options['CURLOPT_MAXREDIRS']) { @@ -3772,6 +3777,12 @@ class curl { if (isset($this->info['redirect_url'])) { if (preg_match('|^https?://|i', $this->info['redirect_url'])) { $redirecturl = $this->info['redirect_url']; + } else { + // Emulate CURLOPT_REDIR_PROTOCOLS behaviour which we have set to (CURLPROTO_HTTP | CURLPROTO_HTTPS) only. + $this->errno = CURLE_UNSUPPORTED_PROTOCOL; + $this->error = 'Redirect to a URL with unsuported protocol: ' . $this->info['redirect_url']; + curl_close($curl); + return $this->error; } } if (!$redirecturl) { @@ -3801,6 +3812,13 @@ class curl { } } + $urlisblocked = $this->check_securityhelper_blocklist($redirecturl); + if (!is_null($urlisblocked)) { + $this->reset_request_state_vars(); + curl_close($curl); + return $urlisblocked; + } + curl_setopt($curl, CURLOPT_URL, $redirecturl); $ret = curl_exec($curl);