Fix invalid internal tag used for postgres dollar quoted. (#3796)

* Fix invalid our internal tag for postgres dollar quoted. * Add test on sql injection with quickwit guard.
quickwit-oss · Sep 4, 2023 · 00f2304 · 00f2304
1 parent ac3b93a
commit 00f2304
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 8 deletions.
diff --git a/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs b/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs
@@ -1238,7 +1238,7 @@ fn generate_dollar_guard(s: &str) -> String {
     }
     let mut dollar_guard = String::new();
     loop {
-        dollar_guard.push_str("Quickwit!");
+        dollar_guard.push_str("QuickwitGuard");
         // This terminates because `dollar_guard`
         // will eventually be longer than s.
         if !s.contains(&dollar_guard) {
@@ -1470,7 +1470,13 @@ mod tests {
         let tags_ast = tag("tag:$$;DELETE FROM something_evil");
         test_tags_filter_expression_helper(
             tags_ast,
-            "$Quickwit!$tag:$$;DELETE FROM something_evil$Quickwit!$ = ANY(tags)",
+            "$QuickwitGuard$tag:$$;DELETE FROM something_evil$QuickwitGuard$ = ANY(tags)",
+        );
+        let tags_ast = tag("tag:$QuickwitGuard$;DELETE FROM something_evil");
+        test_tags_filter_expression_helper(
+            tags_ast,
+            "$QuickwitGuardQuickwitGuard$tag:$QuickwitGuard$;DELETE FROM \
+             something_evil$QuickwitGuardQuickwitGuard$ = ANY(tags)",
         );
     }
 

diff --git a/quickwit/quickwit-metastore/src/tests.rs b/quickwit/quickwit-metastore/src/tests.rs
@@ -1806,7 +1806,7 @@ pub mod test_suite {
             maturity: SplitMaturity::Immature {
                 maturation_period: Duration::from_secs(0),
             },
-            tags: to_btree_set(&["tag!", "tag:foo", "tag:bar"]),
+            tags: to_btree_set(&["tag!", "tag:foo", "$tag!", "$tag:bar"]),
             delete_opstamp: 3,
             ..Default::default()
         };
@@ -1820,7 +1820,7 @@ pub mod test_suite {
             maturity: SplitMaturity::Immature {
                 maturation_period: Duration::from_secs(10),
             },
-            tags: to_btree_set(&["tag!", "tag:bar"]),
+            tags: to_btree_set(&["tag!", "$tag!", "$tag:bar"]),
             delete_opstamp: 1,
             ..Default::default()
         };
@@ -1834,7 +1834,7 @@ pub mod test_suite {
             maturity: SplitMaturity::Immature {
                 maturation_period: Duration::from_secs(20),
             },
-            tags: to_btree_set(&["tag!", "tag:foo", "tag:baz"]),
+            tags: to_btree_set(&["tag!", "tag:foo", "tag:baz", "$tag!"]),
             delete_opstamp: 5,
             ..Default::default()
         };
@@ -1844,7 +1844,7 @@ pub mod test_suite {
             split_id: split_id_4.clone(),
             index_uid: index_uid.clone(),
             time_range: Some(300..=399),
-            tags: to_btree_set(&["tag!", "tag:foo"]),
+            tags: to_btree_set(&["tag!", "tag:foo", "$tag!"]),
             delete_opstamp: 7,
             ..Default::default()
         };
@@ -1855,7 +1855,7 @@ pub mod test_suite {
             index_uid: index_uid.clone(),
             time_range: None,
             create_timestamp: current_timestamp,
-            tags: to_btree_set(&["tag!", "tag:baz", "tag:biz"]),
+            tags: to_btree_set(&["tag!", "tag:baz", "tag:biz", "$tag!"]),
             delete_opstamp: 9,
             ..Default::default()
         };
@@ -2156,7 +2156,7 @@ pub mod test_suite {
             );
 
             let tag_filter_ast = TagFilterAst::Or(vec![
-                TagFilterAst::Or(vec![no_tag("tag!"), tag("tag:bar")]),
+                TagFilterAst::Or(vec![no_tag("$tag!"), tag("$tag:bar")]),
                 TagFilterAst::Or(vec![no_tag("tag!"), tag("tag:baz")]),
             ]);
             let query = ListSplitsQuery::for_index(index_uid.clone())