Merge pull request #9473 from LawnGnome/null-bytes

search: check for null bytes before querying

Author: Turbo87

src/controllers/krate/search.rs

@@ -55,28 +55,29 @@ pub async fn search(app: AppState, req: Parts) -> AppResult<Json<Value>> {
         use seek::*;
 
         let params = req.query();
-        let option_param = |s| params.get(s).map(|v| v.as_str());
-        let sort = option_param("sort");
-        let include_yanked = option_param("include_yanked")
+        let option_param = |s| match params.get(s).map(|v| v.as_str()) {
+            Some(v) if v.contains('\0') => Err(bad_request(format!(
+                "parameter {s} cannot contain a null byte"
+            ))),
+            Some(v) => Ok(Some(v)),
+            None => Ok(None),
+        };
+        let sort = option_param("sort")?;
+        let include_yanked = option_param("include_yanked")?
             .map(|s| s == "yes")
             .unwrap_or(true);
 
-        // Remove 0x00 characters from the query string because Postgres can not
-        // handle them and will return an error, which would cause us to throw
-        // an Internal Server Error ourselves.
-        let q_string = option_param("q").map(|q| q.replace('\u{0}', ""));
-
         let filter_params = FilterParams {
-            q_string: q_string.as_deref(),
+            q_string: option_param("q")?,
             include_yanked,
-            category: option_param("category"),
-            all_keywords: option_param("all_keywords"),
-            keyword: option_param("keyword"),
-            letter: option_param("letter"),
-            user_id: option_param("user_id").and_then(|s| s.parse::<i32>().ok()),
-            team_id: option_param("team_id").and_then(|s| s.parse::<i32>().ok()),
-            following: option_param("following").is_some(),
-            has_ids: option_param("ids[]").is_some(),
+            category: option_param("category")?,
+            all_keywords: option_param("all_keywords")?,
+            keyword: option_param("keyword")?,
+            letter: option_param("letter")?,
+            user_id: option_param("user_id")?.and_then(|s| s.parse::<i32>().ok()),
+            team_id: option_param("team_id")?.and_then(|s| s.parse::<i32>().ok()),
+            following: option_param("following")?.is_some(),
+            has_ids: option_param("ids[]")?.is_some(),
             ..Default::default()
         };
 
@@ -95,7 +96,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult<Json<Value>> {
             .left_join(recent_crate_downloads::table)
             .select(selection);
 
-        if let Some(q_string) = &q_string {
+        if let Some(q_string) = &filter_params.q_string {
             if !q_string.is_empty() {
                 let sort = sort.unwrap_or("relevance");
 

src/tests/routes/crates/list.rs

@@ -193,11 +193,6 @@ async fn index_queries() {
         assert_eq!(cl.crates.len(), 0);
         assert_eq!(cl.meta.total, 0);
     }
-
-    // ignores 0x00 characters that Postgres does not support
-    for cl in search_both(&anon, "q=k%00w1").await {
-        assert_eq!(cl.meta.total, 3);
-    }
 }
 
 #[tokio::test(flavor = "multi_thread")]
@@ -1015,6 +1010,17 @@ async fn test_pages_work_even_with_seek_based_pagination() {
     assert_eq!(second.meta.total, 3);
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn invalid_params_with_null_bytes() {
+    let (_app, anon, _cookie) = TestApp::init().with_user();
+
+    for name in ["q", "category", "all_keywords", "keyword", "letter"] {
+        let response = anon.get::<()>(&format!("/api/v1/crates?{name}=%00")).await;
+        assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+        assert_json_snapshot!(response.json());
+    }
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn invalid_seek_parameter() {
     let (_app, anon, _cookie) = TestApp::init().with_user();

src/tests/routes/crates/snapshots/all__routes__crates__list__invalid_params_with_null_bytes-2.snap

@@ -0,0 +1,11 @@
+---
+source: src/tests/routes/crates/list.rs
+expression: response.json()
+---
+{
+  "errors": [
+    {
+      "detail": "parameter category cannot contain a null byte"
+    }
+  ]
+}

src/tests/routes/crates/snapshots/all__routes__crates__list__invalid_params_with_null_bytes-3.snap

@@ -0,0 +1,11 @@
+---
+source: src/tests/routes/crates/list.rs
+expression: response.json()
+---
+{
+  "errors": [
+    {
+      "detail": "parameter all_keywords cannot contain a null byte"
+    }
+  ]
+}

src/tests/routes/crates/snapshots/all__routes__crates__list__invalid_params_with_null_bytes-4.snap

@@ -0,0 +1,11 @@
+---
+source: src/tests/routes/crates/list.rs
+expression: response.json()
+---
+{
+  "errors": [
+    {
+      "detail": "parameter keyword cannot contain a null byte"
+    }
+  ]
+}

src/tests/routes/crates/snapshots/all__routes__crates__list__invalid_params_with_null_bytes-5.snap

@@ -0,0 +1,11 @@
+---
+source: src/tests/routes/crates/list.rs
+expression: response.json()
+---
+{
+  "errors": [
+    {
+      "detail": "parameter letter cannot contain a null byte"
+    }
+  ]
+}

src/tests/routes/crates/snapshots/all__routes__crates__list__invalid_params_with_null_bytes.snap

@@ -0,0 +1,11 @@
+---
+source: src/tests/routes/crates/list.rs
+expression: response.json()
+---
+{
+  "errors": [
+    {
+      "detail": "parameter q cannot contain a null byte"
+    }
+  ]
+}