diff --git a/enrol/ldap/lib.php b/enrol/ldap/lib.php index 5b9a4cf18aa8a..dbf0e4e68402a 100644 --- a/enrol/ldap/lib.php +++ b/enrol/ldap/lib.php @@ -708,6 +708,7 @@ protected function find_ext_enrolments($memberuid, $role) { $usergroups = $this->ldap_find_user_groups($extmemberuid); if(count($usergroups) > 0) { foreach ($usergroups as $group) { + $group = ldap_filter_addslashes($group); $ldap_search_pattern .= '('.$this->get_config('memberattribute_role'.$role->id).'='.$group.')'; } } diff --git a/lib/ldaplib.php b/lib/ldaplib.php index 31ef0e181a1cf..8e5e3fee5c940 100644 --- a/lib/ldaplib.php +++ b/lib/ldaplib.php @@ -327,6 +327,9 @@ function ldap_filter_addslashes($text) { if(!defined('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA')) { define('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA', 2); } +if(!defined('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX')) { + define('LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX', 3); +} /** * The order of the special characters in these arrays _IS IMPORTANT_. @@ -334,22 +337,36 @@ function ldap_filter_addslashes($text) { * Otherwise we'll double replace '\' with '\5C' which is Bad(tm) */ function ldap_get_dn_special_chars() { - return array ( + static $specialchars = null; + + if ($specialchars !== null) { + return $specialchars; + } + + $specialchars = array ( LDAP_DN_SPECIAL_CHARS => array('\\', ' ', '"', '#', '+', ',', ';', '<', '=', '>', "\0"), LDAP_DN_SPECIAL_CHARS_QUOTED_NUM => array('\\5c','\\20','\\22','\\23','\\2b','\\2c','\\3b','\\3c','\\3d','\\3e','\\00'), - LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA => array('\\\\','\\ ', '\\"', '\\#', '\\+', '\\,', '\\;', '\\<', '\\>', '\\=', '\\00'), + LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA => array('\\\\','\\ ', '\\"', '\\#', '\\+', '\\,', '\\;', '\\<', '\\=', '\\>', '\\00'), ); + $alpharegex = implode('|', array_map (function ($expr) { return preg_quote($expr); }, + $specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA])); + $specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX] = $alpharegex; + + return $specialchars; } /** - * Quote control characters in distinguished names used in LDAP - See RFC 4514/2253 + * Quote control characters in AttributeValue parts of a RelativeDistinguishedName + * used in LDAP distinguished names - See RFC 4514/2253 * - * @param string The text to quote - * @return string The text quoted + * @param string the AttributeValue to quote + * @return string the AttributeValue quoted */ function ldap_addslashes($text) { $special_dn_chars = ldap_get_dn_special_chars(); + // Use the preferred/universal quotation method: ESC HEX HEX + // (i.e., the 'numerically' quoted characters) $text = str_replace ($special_dn_chars[LDAP_DN_SPECIAL_CHARS], $special_dn_chars[LDAP_DN_SPECIAL_CHARS_QUOTED_NUM], $text); @@ -357,25 +374,34 @@ function ldap_addslashes($text) { } /** - * Unquote control characters in distinguished names used in LDAP - See RFC 4514/2253 + * Unquote control characters in AttributeValue parts of a RelativeDistinguishedName + * used in LDAP distinguished names - See RFC 4514/2253 * - * @param string The text quoted - * @return string The text unquoted + * @param string the AttributeValue quoted + * @return string the AttributeValue unquoted */ function ldap_stripslashes($text) { - $special_dn_chars = ldap_get_dn_special_chars(); - - // First unquote the simply backslashed special characters. If we - // do it the other way, we remove too many slashes. - $text = str_replace($special_dn_chars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA], - $special_dn_chars[LDAP_DN_SPECIAL_CHARS], - $text); + $specialchars = ldap_get_dn_special_chars(); - // Next unquote the 'numerically' quoted characters. We don't use - // LDAP_DN_SPECIAL_CHARS_QUOTED_NUM because the standard allows us - // to quote any character with this encoding, not just the special + // We can't unquote in two steps, as we end up unquoting too much in certain cases. So + // we need to build a regexp containing both the 'numerically' and 'alphabetically' + // quoted characters. We don't use LDAP_DN_SPECIAL_CHARS_QUOTED_NUM because the + // standard allows us to quote any character with this encoding, not just the special // ones. - $text = preg_replace('/\\\([0-9A-Fa-f]{2})/e', "chr(hexdec('\\1'))", $text); + // @TODO: This still misses some special (and rarely used) cases, but we need + // a full state machine to handle them. + $quoted = '/(\\\\[0-9A-Fa-f]{2}|' . $specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA_REGEX] . ')/'; + $text = preg_replace_callback($quoted, + function ($match) use ($specialchars) { + if (ctype_xdigit(ltrim($match[1], '\\'))) { + return chr(hexdec($match[1])); + } else { + return str_replace($specialchars[LDAP_DN_SPECIAL_CHARS_QUOTED_ALPHA], + $specialchars[LDAP_DN_SPECIAL_CHARS], + $match[1]); + } + }, + $text); return $text; } diff --git a/lib/tests/ldaplib_test.php b/lib/tests/ldaplib_test.php new file mode 100644 index 0000000000000..21ab25447710b --- /dev/null +++ b/lib/tests/ldaplib_test.php @@ -0,0 +1,169 @@ +. + +/** + * ldap tests. + * + * @package core + * @category phpunit + * @copyright Damyon Wiese, Iñaki Arenaza 2014 + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->libdir . '/ldaplib.php'); + +class core_ldaplib_testcase extends advanced_testcase { + + public function test_ldap_addslashes() { + // See http://tools.ietf.org/html/rfc4514#section-5.2 if you want + // to add additional tests. + + $tests = array( + array ( + 'test' => 'Simplest', + 'expected' => 'Simplest', + ), + array ( + 'test' => 'Simple case', + 'expected' => 'Simple\\20case', + ), + array ( + 'test' => 'Medium ‒ case', + 'expected' => 'Medium\\20‒\\20case', + ), + array ( + 'test' => '#Harder+case#', + 'expected' => '\\23Harder\\2bcase\\23', + ), + array ( + 'test' => ' Harder (and); harder case ', + 'expected' => '\\20Harder\\20(and)\\3b\\20harder\\20case\\20', + ), + array ( + 'test' => 'Really \\0 (hard) case!\\', + 'expected' => 'Really\\20\\5c0\\20(hard)\\20case!\\5c', + ), + array ( + 'test' => 'James "Jim" = Smith, III', + 'expected' => 'James\\20\\22Jim\22\\20\\3d\\20Smith\\2c\\20III', + ), + array ( + 'test' => ' ', + 'expected' => '\\20\\20\\3cjsmith@test.local\\3e\\20', + ), + ); + + + foreach ($tests as $test) { + $this->assertSame($test['expected'], ldap_addslashes($test['test'])); + } + } + + public function test_ldap_stripslashes() { + // See http://tools.ietf.org/html/rfc4514#section-5.2 if you want + // to add additional tests. + + // IMPORTANT NOTICE: While ldap_addslashes() only produces one + // of the two defined ways of escaping/quoting (the ESC HEX + // HEX way defined in the grammar in Section 3 of RFC-4514) + // ldap_stripslashes() has to deal with both of them. So in + // addition to testing the same strings we test in + // test_ldap_stripslashes(), we need to also test strings + // using the second method. + + $tests = array( + array ( + 'test' => 'Simplest', + 'expected' => 'Simplest', + ), + array ( + 'test' => 'Simple\\20case', + 'expected' => 'Simple case', + ), + array ( + 'test' => 'Simple\\ case', + 'expected' => 'Simple case', + ), + array ( + 'test' => 'Simple\\ \\63\\61\\73\\65', + 'expected' => 'Simple case', + ), + array ( + 'test' => 'Medium\\ ‒\\ case', + 'expected' => 'Medium ‒ case', + ), + array ( + 'test' => 'Medium\\20‒\\20case', + 'expected' => 'Medium ‒ case', + ), + array ( + 'test' => 'Medium\\20\\E2\\80\\92\\20case', + 'expected' => 'Medium ‒ case', + ), + array ( + 'test' => '\\23Harder\\2bcase\\23', + 'expected' => '#Harder+case#', + ), + array ( + 'test' => '\\#Harder\\+case\\#', + 'expected' => '#Harder+case#', + ), + array ( + 'test' => '\\20Harder\\20(and)\\3b\\20harder\\20case\\20', + 'expected' => ' Harder (and); harder case ', + ), + array ( + 'test' => '\\ Harder\\ (and)\\;\\ harder\\ case\\ ', + 'expected' => ' Harder (and); harder case ', + ), + array ( + 'test' => 'Really\\20\\5c0\\20(hard)\\20case!\\5c', + 'expected' => 'Really \\0 (hard) case!\\', + ), + array ( + 'test' => 'Really\\ \\\\0\\ (hard)\\ case!\\\\', + 'expected' => 'Really \\0 (hard) case!\\', + ), + array ( + 'test' => 'James\\20\\22Jim\\22\\20\\3d\\20Smith\\2c\\20III', + 'expected' => 'James "Jim" = Smith, III', + ), + array ( + 'test' => 'James\\ \\"Jim\\" \\= Smith\\, III', + 'expected' => 'James "Jim" = Smith, III', + ), + array ( + 'test' => '\\20\\20\\3cjsmith@test.local\\3e\\20', + 'expected' => ' ', + ), + array ( + 'test' => '\\ \\\\ ', + 'expected' => ' ', + ), + array ( + 'test' => 'Lu\\C4\\8Di\\C4\\87', + 'expected' => 'Lučić', + ), + ); + + foreach ($tests as $test) { + $this->assertSame($test['expected'], ldap_stripslashes($test['test'])); + } + } +}