CVE-2024-3078: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: A vulnerability was found in Qdrant up to 1.6.1/1.7.4/1.8.2 and classified as critical. This issue affects some unknown processing of the file lib/collection/src/collection/snapshots.rs of the component Full Snapshot REST API. The manipulation leads to path traversal. Upgrading to version 1.8.3 is able to address this issue. The patch is named 3ab5172e9c8f14fa1f7b24e7147eac74e2412b62. It is recommended to upgrade the affected component. The associated identifier of this vulnerability is VDB-258611.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
Published: 2024-03-29T13:15Z
Repository: https://github.com/qdrant/qdrant
Language: Rust
Files changed: lib/collection/src/collection/snapshots.rs, lib/collection/src/operations/types.rs, lib/storage/src/content_manager/errors.rs, lib/storage/src/content_manager/snapshots/mod.rs, tests/openapi/openapi_integration/test_snapshot.py
Commit date: 2024-03-19 13:31:45 +0100
Commit message: Fix arbitrary path traversal vulnerability in full snapshot REST API (#3856)

* Improve function to get collection snapshot path

* Fix arbitrary path traversal in full snapshot path function

* Add integration test to assert arbitrary path traversal is blocked
Diff: diff --git a/lib/collection/src/collection/snapshots.rs b/lib/collection/src/collection/snapshots.rs
index b626fa11a..356898d62 100644
--- a/lib/collection/src/collection/snapshots.rs
+++ b/lib/collection/src/collection/snapshots.rs
@@ -230,39 +230,39 @@ impl Collection {
             .await
             .recover_local_shard_from(snapshot_shard_path, shard_id, cancel)
             .await
     }
 
+    /// Get full file path for a collection snapshot by name
+    ///
+    /// This enforces the file to be inside the snapshots directory
     pub async fn get_snapshot_path(&self, snapshot_name: &str) -> CollectionResult<PathBuf> {
-        let snapshot_path = self.snapshots_path.join(snapshot_name);
-
-        let absolute_snapshot_path =
-            snapshot_path
-                .canonicalize()
-                .map_err(|_| CollectionError::NotFound {
-                    what: format!("Snapshot {snapshot_name}"),
-                })?;
-
-        let absolute_snapshot_dir =
-            self.snapshots_path
-                .canonicalize()
-                .map_err(|_| CollectionError::NotFound {
-                    what: format!("Snapshot directory: {}", self.snapshots_path.display()),
-                })?;
+        let absolute_snapshot_dir = self.snapshots_path.canonicalize().map_err(|_| {
+            CollectionError::not_found(format!(
+                "Snapshot directory: {}",
+                self.snapshots_path.display()
+            ))
+        })?;
+
+        let absolute_snapshot_path = absolute_snapshot_dir
+            .join(snapshot_name)
+            .canonicalize()
+            .map_err(|_| CollectionError::not_found(format!("Snapshot {snapshot_name}")))?;
 
         if !absolute_snapshot_path.starts_with(absolute_snapshot_dir) {
-            return Err(CollectionError::NotFound {
-                what: format!("Snapshot {snapshot_name}"),
-            });
+            return Err(CollectionError::not_found(format!(
+                "Snapshot {snapshot_name}"
+            )));
         }
 
-        if !snapshot_path.exists() {
-            return Err(CollectionError::NotFound {
-                what: format!("Snapshot {snapshot_name}"),
-            });
+        if !absolute_snapshot_path.exists() {
+            return Err(CollectionError::not_found(format!(
+                "Snapshot {snapshot_name}"
+            )));
         }
-        Ok(snapshot_path)
+
+        Ok(absolute_snapshot_path)
     }
 
     pub async fn list_shard_snapshots(
         &self,
         shard_id: ShardId,
diff --git a/lib/collection/src/operations/types.rs b/lib/collection/src/operations/types.rs
index eeaf07850..10525ecd8 100644
--- a/lib/collection/src/operations/types.rs
+++ b/lib/collection/src/operations/types.rs
@@ -957,10 +957,14 @@ impl CollectionError {
 
     pub fn bad_input(description: String) -> CollectionError {
         CollectionError::BadInput { description }
     }
 
+    pub fn not_found(what: impl Into<String>) -> CollectionError {
+        CollectionError::NotFound { what: what.into() }
+    }
+
     pub fn bad_request(description: String) -> CollectionError {
         CollectionError::BadRequest { description }
     }
 
     pub fn bad_shard_selection(description: String) -> CollectionError {
diff --git a/lib/storage/src/content_manager/errors.rs b/lib/storage/src/content_manager/errors.rs
index ed496ec6d..93bf8d2a7 100644
--- a/lib/storage/src/content_manager/errors.rs
+++ b/lib/storage/src/content_manager/errors.rs
@@ -54,10 +54,16 @@ impl StorageError {
         StorageError::AlreadyExists {
             description: description.into(),
         }
     }
 
+    pub fn not_found(description: impl Into<String>) -> StorageError {
+        StorageError::NotFound {
+            description: description.into(),
+        }
+    }
+
     pub fn checksum_mismatch(expected: impl Into<String>, actual: impl Into<String>) -> Self {
         StorageError::ChecksumMismatch {
             expected: expected.into(),
             actual: actual.into(),
         }
diff --git a/lib/storage/src/content_manager/snapshots/mod.rs b/lib/storage/src/content_manager/snapshots/mod.rs
index 2f7ec2079..4c0e771a2 100644
--- a/lib/storage/src/content_manager/snapshots/mod.rs
+++ b/lib/storage/src/content_manager/snapshots/mod.rs
@@ -21,21 +21,37 @@ pub struct SnapshotConfig {
     /// Aliases for collections `<alias>:<collection_name>`
     #[serde(default)]
     pub collections_aliases: HashMap<String, String>,
 }
 
+/// Get full file path for a full snapshot by name
+///
+/// This enforces the file to be inside the snapshots directory
 pub async fn get_full_snapshot_path(
     toc: &TableOfContent,
     snapshot_name: &str,
 ) -> Result<PathBuf, StorageError> {
-    let snapshot_path = Path::new(toc.snapshots_path()).join(snapshot_name);
-    if !snapshot_path.exists() {
-        return Err(StorageError::NotFound {
-            description: format!("Full storage snapshot {snapshot_name} not found"),
-        });
+    let snapshots_path = toc.snapshots_path();
+
+    let absolute_snapshot_dir = Path::new(snapshots_path)
+        .canonicalize()
+        .map_err(|_| StorageError::not_found(format!("Snapshot directory: {snapshots_path}")))?;
+
+    let absolute_snapshot_path = absolute_snapshot_dir
+        .join(snapshot_name)
+        .canonicalize()
+        .map_err(|_| StorageError::not_found(format!("Snapshot {snapshot_name}")))?;
+
+    if !absolute_snapshot_path.starts_with(absolute_snapshot_dir) {
+        return Err(StorageError::not_found(format!("Snapshot {snapshot_name}")));
     }
-    Ok(snapshot_path)
+
+    if !absolute_snapshot_path.exists() {
+        return Err(StorageError::not_found(format!("Snapshot {snapshot_name}")));
+    }
+
+    Ok(absolute_snapshot_path)
 }
 
 pub async fn do_delete_full_snapshot(
     dispatcher: &Dispatcher,
     snapshot_name: &str,
diff --git a/tests/openapi/openapi_integration/test_snapshot.py b/tests/openapi/openapi_integration/test_snapshot.py
index 4c83b421a..6f3ebccc6 100644
--- a/tests/openapi/openapi_integration/test_snapshot.py
+++ b/tests/openapi/openapi_integration/test_snapshot.py
@@ -1,12 +1,16 @@
 from time import sleep
 import hashlib
+import os
 import pytest
+import requests
 
 from .helpers.collection_setup import basic_collection_setup, drop_collection
 from .helpers.helpers import request_with_validation
 
+QDRANT_HOST = os.environ.get("QDRANT_HOST", "localhost:6333")
+
 collection_name = 'test_collection_snapshot'
 
 
 @pytest.fixture(autouse=True)
 def setup(on_disk_vectors):
@@ -359,5 +363,33 @@ def test_snapshot_invalid_file_uri():
             "location": "file://localhost/whatever.snapshot",
         }
     )
     assert response.status_code == 400
     assert response.json()["status"]["error"] == "Bad request: Snapshot file \"/whatever.snapshot\" does not exist"
+
+
+def test_security():
+    # ensure we cannot do simple arbitrary path traversal
+    name = "/etc/passwd"
+    response = requests.get(
+        f"http://{QDRANT_HOST}/collections/{name}",
+        headers={"Content-Type": "application/json"},
+    )
+    assert not response.ok
+    assert response.status_code == 404
+
+    name = "../../../../../../../etc/passwd"
+    response = requests.get(
+        f"http://{QDRANT_HOST}/collections/{name}",
+        headers={"Content-Type": "application/json"},
+    )
+    assert not response.ok
+    assert response.status_code == 404
+
+    # ensure we cannot do arbitrary path traversal with encoded slashes
+    response = request_with_validation(
+        api='/snapshots/{snapshot_name}',
+        path_params={'snapshot_name': '..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2Fetc%2Fpasswd'},
+        method="GET",
+    )
+    assert not response.ok
+    assert response.status_code == 404

Vulnerable code:         let snapshot_path = self.snapshots_path.join(snapshot_name);

        let absolute_snapshot_path =
            snapshot_path
                .canonicalize()
                .map_err(|_| CollectionError::NotFound {
                    what: format!("Snapshot {snapshot_name}"),
                })?;

        let absolute_snapshot_dir =
            self.snapshots_path
                .canonicalize()
                .map_err(|_| CollectionError::NotFound {
                    what: format!("Snapshot directory: {}", self.snapshots_path.display()),
                })?;
            return Err(CollectionError::NotFound {
                what: format!("Snapshot {snapshot_name}"),
            });
        if !snapshot_path.exists() {
            return Err(CollectionError::NotFound {
                what: format!("Snapshot {snapshot_name}"),
            });
        Ok(snapshot_path)
    let snapshot_path = Path::new(toc.snapshots_path()).join(snapshot_name);
    if !snapshot_path.exists() {
        return Err(StorageError::NotFound {
            description: format!("Full storage snapshot {snapshot_name} not found"),
        });
    Ok(snapshot_path)
Fixed code:     /// Get full file path for a collection snapshot by name
    ///
    /// This enforces the file to be inside the snapshots directory
        let absolute_snapshot_dir = self.snapshots_path.canonicalize().map_err(|_| {
            CollectionError::not_found(format!(
                "Snapshot directory: {}",
                self.snapshots_path.display()
            ))
        })?;

        let absolute_snapshot_path = absolute_snapshot_dir
            .join(snapshot_name)
            .canonicalize()
            .map_err(|_| CollectionError::not_found(format!("Snapshot {snapshot_name}")))?;
            return Err(CollectionError::not_found(format!(
                "Snapshot {snapshot_name}"
            )));
        if !absolute_snapshot_path.exists() {
            return Err(CollectionError::not_found(format!(
                "Snapshot {snapshot_name}"
            )));

        Ok(absolute_snapshot_path)
    pub fn not_found(what: impl Into<String>) -> CollectionError {
        CollectionError::NotFound { what: what.into() }
    }

    pub fn not_found(description: impl Into<String>) -> StorageError {
        StorageError::NotFound {
            description: description.into(),
        }
    }

/// Get full file path for a full snapshot by name
///
/// This enforces the file to be inside the snapshots directory
    let snapshots_path = toc.snapshots_path();

    let absolute_snapshot_dir = Path::new(snapshots_path)
        .canonicalize()
        .map_err(|_| StorageError::not_found(format!("Snapshot directory: {snapshots_path}")))?;

    let absolute_snapshot_path = absolute_snapshot_dir
        .join(snapshot_name)
        .canonicalize()
        .map_err(|_| StorageError::not_found(format!("Snapshot {snapshot_name}")))?;

    if !absolute_snapshot_path.starts_with(absolute_snapshot_dir) {
        return Err(StorageError::not_found(format!("Snapshot {snapshot_name}")));

    if !absolute_snapshot_path.exists() {
        return Err(StorageError::not_found(format!("Snapshot {snapshot_name}")));
    }

    Ok(absolute_snapshot_path)
import os
import requests
QDRANT_HOST = os.environ.get("QDRANT_HOST", "localhost:6333")



def test_security():
    # ensure we cannot do simple arbitrary path traversal
    name = "/etc/passwd"
    response = requests.get(
        f"http://{QDRANT_HOST}/collections/{name}",
        headers={"Content-Type": "application/json"},
    )
    assert not response.ok
    assert response.status_code == 404

    name = "../../../../../../../etc/passwd"
    response = requests.get(
        f"http://{QDRANT_HOST}/collections/{name}",
        headers={"Content-Type": "application/json"},
    )
    assert not response.ok
    assert response.status_code == 404

    # ensure we cannot do arbitrary path traversal with encoded slashes
    response = request_with_validation(
        api='/snapshots/{snapshot_name}',
        path_params={'snapshot_name': '..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2F..%2Fetc%2Fpasswd'},
        method="GET",
    )
    assert not response.ok
    assert response.status_code == 404