Do not accept pattern_type from user input in push rules. (#15088)

Internally the push rules module uses a `pattern_type` property for `event_match`
conditions (and `related_event_match`) to mark the condition as matching the
current user's Matrix ID or localpart.

This is leaky to the Client-Server API where a user can successfully set a condition
which provides `pattern_type` instead of `pattern` (note that there's no benefit to
doing this -- the user can just use their own Matrix ID or localpart instead). When
serializing back to the client the `pattern_type` property is converted into a proper
`pattern`.

The following changes are made to avoid this:

* Separate the `KnownCondition::EventMatch` enum value into `EventMatch`
  and `EventMatchType`, each with their own expected properties. (Note that a
  similar change is made for `RelatedEventMatch`.)
* Make it such that the `pattern_type` variants serialize to the same condition kind,
  but cannot be deserialized (since they're only provided by base rules).
* As a final tweak, convert `user_id` vs. `user_localpart` values into an enum.

1 files changed, 71 insertions, 84 deletions

diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs
index 55551ecb56..a65c645caf 100644
--- a/rust/src/push/evaluator.rs
+++ b/rust/src/push/evaluator.rs
@@ -12,9 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::borrow::Cow;
 use std::collections::{BTreeMap, BTreeSet};
 
-use crate::push::JsonValue;
+use crate::push::{EventMatchPatternType, JsonValue};
 use anyhow::{Context, Error};
 use lazy_static::lazy_static;
 use log::warn;
@@ -23,8 +24,8 @@ use regex::Regex;
 
 use super::{
     utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType},
-    Action, Condition, EventMatchCondition, ExactEventMatchCondition, FilteredPushRules,
-    KnownCondition, RelatedEventMatchCondition, SimpleJsonValue,
+    Action, Condition, ExactEventMatchCondition, FilteredPushRules, KnownCondition,
+    SimpleJsonValue,
 };
 
 lazy_static! {
@@ -256,14 +257,58 @@ impl PushRuleEvaluator {
         };
 
         let result = match known_condition {
-            KnownCondition::EventMatch(event_match) => {
-                self.match_event_match(event_match, user_id)?
+            KnownCondition::EventMatch(event_match) => self.match_event_match(
+                &self.flattened_keys,
+                &event_match.key,
+                &event_match.pattern,
+            )?,
+            KnownCondition::EventMatchType(event_match) => {
+                // The `pattern_type` can either be "user_id" or "user_localpart",
+                // either way if we don't have a `user_id` then the condition can't
+                // match.
+                let user_id = if let Some(user_id) = user_id {
+                    user_id
+                } else {
+                    return Ok(false);
+                };
+
+                let pattern = match &*event_match.pattern_type {
+                    EventMatchPatternType::UserId => user_id,
+                    EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?,
+                };
+
+                self.match_event_match(&self.flattened_keys, &event_match.key, pattern)?
             }
             KnownCondition::ExactEventMatch(exact_event_match) => {
                 self.match_exact_event_match(exact_event_match)?
             }
-            KnownCondition::RelatedEventMatch(event_match) => {
-                self.match_related_event_match(event_match, user_id)?
+            KnownCondition::RelatedEventMatch(event_match) => self.match_related_event_match(
+                &event_match.rel_type.clone(),
+                event_match.include_fallbacks,
+                event_match.key.clone(),
+                event_match.pattern.clone(),
+            )?,
+            KnownCondition::RelatedEventMatchType(event_match) => {
+                // The `pattern_type` can either be "user_id" or "user_localpart",
+                // either way if we don't have a `user_id` then the condition can't
+                // match.
+                let user_id = if let Some(user_id) = user_id {
+                    user_id
+                } else {
+                    return Ok(false);
+                };
+
+                let pattern = match &*event_match.pattern_type {
+                    EventMatchPatternType::UserId => user_id,
+                    EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?,
+                };
+
+                self.match_related_event_match(
+                    &event_match.rel_type.clone(),
+                    event_match.include_fallbacks,
+                    Some(event_match.key.clone()),
+                    Some(Cow::Borrowed(pattern)),
+                )?
             }
             KnownCondition::ExactEventPropertyContains(exact_event_match) => {
                 self.match_exact_event_property_contains(exact_event_match)?
@@ -325,32 +370,12 @@ impl PushRuleEvaluator {
     /// Evaluates a `event_match` condition.
     fn match_event_match(
         &self,
-        event_match: &EventMatchCondition,
-        user_id: Option<&str>,
+        flattened_event: &BTreeMap<String, JsonValue>,
+        key: &str,
+        pattern: &str,
     ) -> Result<bool, Error> {
-        let pattern = if let Some(pattern) = &event_match.pattern {
-            pattern
-        } else if let Some(pattern_type) = &event_match.pattern_type {
-            // The `pattern_type` can either be "user_id" or "user_localpart",
-            // either way if we don't have a `user_id` then the condition can't
-            // match.
-            let user_id = if let Some(user_id) = user_id {
-                user_id
-            } else {
-                return Ok(false);
-            };
-
-            match &**pattern_type {
-                "user_id" => user_id,
-                "user_localpart" => get_localpart_from_id(user_id)?,
-                _ => return Ok(false),
-            }
-        } else {
-            return Ok(false);
-        };
-
         let haystack = if let Some(JsonValue::Value(SimpleJsonValue::Str(haystack))) =
-            self.flattened_keys.get(&*event_match.key)
+            flattened_event.get(key)
         {
             haystack
         } else {
@@ -359,7 +384,7 @@ impl PushRuleEvaluator {
 
         // For the content.body we match against "words", but for everything
         // else we match against the entire value.
-        let match_type = if event_match.key == "content.body" {
+        let match_type = if key == "content.body" {
             GlobMatchType::Word
         } else {
             GlobMatchType::Whole
@@ -395,8 +420,10 @@ impl PushRuleEvaluator {
     /// Evaluates a `related_event_match` condition. (MSC3664)
     fn match_related_event_match(
         &self,
-        event_match: &RelatedEventMatchCondition,
-        user_id: Option<&str>,
+        rel_type: &str,
+        include_fallbacks: Option<bool>,
+        key: Option<Cow<str>>,
+        pattern: Option<Cow<str>>,
     ) -> Result<bool, Error> {
         // First check if related event matching is enabled...
         if !self.related_event_match_enabled {
@@ -404,7 +431,7 @@ impl PushRuleEvaluator {
         }
 
         // get the related event, fail if there is none.
-        let event = if let Some(event) = self.related_events_flattened.get(&*event_match.rel_type) {
+        let event = if let Some(event) = self.related_events_flattened.get(rel_type) {
             event
         } else {
             return Ok(false);
@@ -412,58 +439,18 @@ impl PushRuleEvaluator {
 
         // If we are not matching fallbacks, don't match if our special key indicating this is a
         // fallback relation is not present.
-        if !event_match.include_fallbacks.unwrap_or(false)
-            && event.contains_key("im.vector.is_falling_back")
-        {
+        if !include_fallbacks.unwrap_or(false) && event.contains_key("im.vector.is_falling_back") {
             return Ok(false);
         }
 
-        // if we have no key, accept the event as matching, if it existed without matching any
-        // fields.
-        let key = if let Some(key) = &event_match.key {
-            key
-        } else {
-            return Ok(true);
-        };
-
-        let pattern = if let Some(pattern) = &event_match.pattern {
-            pattern
-        } else if let Some(pattern_type) = &event_match.pattern_type {
-            // The `pattern_type` can either be "user_id" or "user_localpart",
-            // either way if we don't have a `user_id` then the condition can't
-            // match.
-            let user_id = if let Some(user_id) = user_id {
-                user_id
-            } else {
-                return Ok(false);
-            };
-
-            match &**pattern_type {
-                "user_id" => user_id,
-                "user_localpart" => get_localpart_from_id(user_id)?,
-                _ => return Ok(false),
-            }
-        } else {
-            return Ok(false);
-        };
-
-        let haystack =
-            if let Some(JsonValue::Value(SimpleJsonValue::Str(haystack))) = event.get(&**key) {
-                haystack
-            } else {
-                return Ok(false);
-            };
-
-        // For the content.body we match against "words", but for everything
-        // else we match against the entire value.
-        let match_type = if key == "content.body" {
-            GlobMatchType::Word
-        } else {
-            GlobMatchType::Whole
-        };
-
-        let mut compiled_pattern = get_glob_matcher(pattern, match_type)?;
-        compiled_pattern.is_match(haystack)
+        match (key, pattern) {
+            // if we have no key, accept the event as matching.
+            (None, _) => Ok(true),
+            // There was a key, so we *must* have a pattern to go with it.
+            (Some(_), None) => Ok(false),
+            // If there is a key & pattern, check if they're in the flattened event (given by rel_type).
+            (Some(key), Some(pattern)) => self.match_event_match(event, &key, &pattern),
+        }
     }
 
     /// Evaluates a `exact_event_property_contains` condition. (MSC3758)