From 7f16ead05c1c99764d4c83ae55ba576fb9896a14 Mon Sep 17 00:00:00 2001
From: Julian Rother <julian@jrother.eu>
Date: Sun, 5 Dec 2021 03:23:02 +0100
Subject: [PATCH] Fix: Error when matching strings with prohibited characters

An AttributeError was raised in match_equal/match_substr of string matching
rules if any attribute value contained a prohibited character. These attribute
values are now ignored for matching.
---
 ldapserver/schema/matching_rules.py | 30 ++++++++++++-----------------
 tests/test_schema_matching_rule.py  |  7 +++++++
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/ldapserver/schema/matching_rules.py b/ldapserver/schema/matching_rules.py
index 851eb6f..719a119 100644
--- a/ldapserver/schema/matching_rules.py
+++ b/ldapserver/schema/matching_rules.py
@@ -50,32 +50,30 @@ class StringMatchingRuleDefinition(MatchingRuleDefinition):
 		except ValueError as exc:
 			raise exceptions.LDAPInvalidAttributeSyntax('Assertion value contains characters prohibited by RFC4518') from exc
 
-	def prepare_attribute_value(self, attribute_value):
-		try:
-			return rfc4518_stringprep.prepare(attribute_value, self.matching_type)
-		except ValueError:
-			return None
+	def prepare_attribute_values(self, attribute_values):
+		for attribute_value in attribute_values:
+			try:
+				yield rfc4518_stringprep.prepare(attribute_value, self.matching_type)
+			except ValueError:
+				pass
 
 	def match_equal(self, schema, attribute_values, assertion_value):
 		assertion_value = self.prepare_assertion_value(assertion_value)
-		for attribute_value in attribute_values:
-			attribute_value = self.prepare_attribute_value(attribute_value)
+		for attribute_value in self.prepare_attribute_values(attribute_values):
 			if attribute_value == assertion_value:
 				return True
 		return False
 
 	def match_less(self, schema, attribute_values, assertion_value):
 		assertion_value = self.prepare_assertion_value(assertion_value)
-		for attribute_value in attribute_values:
-			attribute_value = self.prepare_attribute_value(attribute_value)
+		for attribute_value in self.prepare_attribute_values(attribute_values):
 			if attribute_value < assertion_value:
 				return True
 		return False
 
 	def match_greater_or_equal(self, schema, attribute_values, assertion_value):
 		assertion_value = self.prepare_assertion_value(assertion_value)
-		for attribute_value in attribute_values:
-			attribute_value = self.prepare_attribute_value(attribute_value)
+		for attribute_value in self.prepare_attribute_values(attribute_values):
 			if attribute_value >= assertion_value:
 				return True
 		return False
@@ -89,13 +87,9 @@ class StringMatchingRuleDefinition(MatchingRuleDefinition):
 				final_substring = rfc4518_stringprep.prepare(final_substring, self.matching_type, rfc4518_stringprep.SubstringType.FINAL)
 		except ValueError as exc:
 			raise exceptions.LDAPInvalidAttributeSyntax('Assertion value contains characters prohibited by RFC4518') from exc
-		for attribute_value in attribute_values:
-			try:
-				attribute_value = self.prepare_attribute_value(attribute_value)
-				if _substr_match(attribute_value, inital_substring, any_substrings, final_substring):
-					return True
-			except ValueError:
-				pass
+		for attribute_value in self.prepare_attribute_values(attribute_values):
+			if _substr_match(attribute_value, inital_substring, any_substrings, final_substring):
+				return True
 		return False
 
 class StringListMatchingRuleDefinition(MatchingRuleDefinition):
diff --git a/tests/test_schema_matching_rule.py b/tests/test_schema_matching_rule.py
index a740ed0..8ce02eb 100644
--- a/tests/test_schema_matching_rule.py
+++ b/tests/test_schema_matching_rule.py
@@ -60,6 +60,10 @@ class TestStringEqualityMatchingRule(unittest.TestCase):
 		self.assertTrue(rule.match_equal(None, ['fo  o ', ' bar'], '   bar   '))
 		self.assertFalse(rule.match_equal(None, ['fo  o ', ' b ar'], '   bar   '))
 		self.assertTrue(rule.match_equal(None, ['fo\n\ro ', ' bar'], ' fo o'))
+		# Prohibited characters make an attribute value unmatchable, but should not cause errors
+		with self.assertRaises(ldapserver.exceptions.LDAPInvalidAttributeSyntax):
+			rule.match_equal(None, ['foobar\uFFFD', 'test'], 'foobar\uFFFD')
+		self.assertTrue(rule.match_equal(None, ['foobar\uFFFD', 'test'], 'test'))
 		# Systematic tests for stringprep in test_stringprep.py
 
 class TestStringOrderingMatchingRule(unittest.TestCase):
@@ -124,6 +128,9 @@ class TestStringSubstrMatchingRule(unittest.TestCase):
 		self.assertTrue(rule.match_substr(None, ['abcdefghi'], 'abc', ['ef'], 'ghi'))
 		self.assertTrue(rule.match_substr(None, ['abcdefghi'], 'abc', ['de'], 'ghi'))
 		self.assertTrue(rule.match_substr(None, ['abcdefghi'], 'abc', ['def'], 'hi'))
+		# Prohibited characters make an attribute value unmatchable, but should not cause errors
+		self.assertFalse(rule.match_substr(None, ['foobar\uFFFD', 'test'], 'foobar', [], None))
+		self.assertTrue(rule.match_substr(None, ['foobar\uFFFD', 'test'], 'test', [], None))
 		# TODO: more systematic tests
 
 class TestStringListEqualityMatchingRule(unittest.TestCase):
-- 
GitLab