diff --git a/lib/moodlelib.php b/lib/moodlelib.php index d00252caba9..462aea614be 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -10243,23 +10243,12 @@ function is_proxybypass( $url ) { // Get the possible bypass hosts into an array. $matches = explode( ',', $CFG->proxybypass ); - // Check for a match. - // (IPs need to match the left hand side and hosts the right of the url, - // but we can recklessly check both as there can't be a false +ve). - foreach ($matches as $match) { - $match = trim($match); + // Check for a exact match on the IP or in the domains. + $isdomaininallowedlist = \core\ip_utils::is_domain_in_allowed_list($host, $matches); + $isipinsubnetlist = \core\ip_utils::is_ip_in_subnet_list($host, $CFG->proxybypass, ','); - // Try for IP match (Left side). - $lhs = substr($host, 0, strlen($match)); - if (strcasecmp($match, $lhs)==0) { - return true; - } - - // Try for host match (Right side). - $rhs = substr($host, -strlen($match)); - if (strcasecmp($match, $rhs)==0) { - return true; - } + if ($isdomaininallowedlist || $isipinsubnetlist) { + return true; } // Nothing matched. diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index bc8ac172a8e..1bdc12e732b 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -5522,4 +5522,70 @@ EOT; $this->assertEquals(false, html_is_blank('

.

')); $this->assertEquals(false, html_is_blank('')); } + + /** + * Provider for is_proxybypass + * + * @return array of test cases. + */ + public function is_proxybypass_provider(): array { + + return [ + 'Proxybypass contains the same IP as the beginning of the URL' => [ + 'http://192.168.5.5-fake-app-7f000101.nip.io', + '192.168.5.5, 127.0.0.1', + false + ], + 'Proxybypass contains the last part of the URL' => [ + 'http://192.168.5.5-fake-app-7f000101.nip.io', + 'app-7f000101.nip.io', + false + ], + 'Proxybypass contains the last part of the URL 2' => [ + 'http://store.mydomain.com', + 'mydomain.com', + false + ], + 'Proxybypass contains part of the url' => [ + 'http://myweb.com', + 'store.myweb.com', + false + ], + 'Different IPs used in proxybypass' => [ + 'http://192.168.5.5', + '192.168.5.3', + false + ], + 'Proxybypass and URL matchs' => [ + 'http://store.mydomain.com', + 'store.mydomain.com', + true + ], + 'IP used in proxybypass' => [ + 'http://192.168.5.5', + '192.168.5.5', + true + ], + ]; + } + + /** + * Check if $url matches anything in proxybypass list + * + * Test function {@see is_proxybypass()}. + * @dataProvider is_proxybypass_provider + * @param string $url url to check + * @param string $proxybypass + * @param bool $expected Expected value. + */ + public function test_is_proxybypass(string $url, string $proxybypass, bool $expected): void { + $this->resetAfterTest(); + + global $CFG; + $CFG->proxyhost = '192.168.5.5'; // Test with a fake proxy. + $CFG->proxybypass = $proxybypass; + + $this->assertEquals($expected, is_proxybypass($url)); + } + }