From b557c117247b0def307623b22203bd09c89bf05f Mon Sep 17 00:00:00 2001
From: Mathijs van Veluw <black.dex@gmail.com>
Date: Sun, 11 Aug 2024 19:39:56 +0200
Subject: [PATCH] Fix data disclosure on organization endpoints (#4837)

- All users were able to request organizational details from any org,
  even if they were not a member (anymore).
  Now it will check if that user is a member of the org or not.
- The `/organization/<uuid>/keys` endpoint returned also the private keys.
  This should not be the case. Also, according to the upstream server
  code the endpoint changed, but the clients do not seem to use it.
  I added it anyway just in case they will in the future.
- Also require a valid login before being able to retreve those org
  keys. Upstream does not do this, but i see no reason why not.

Fixes: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-39925
---
 src/api/core/organizations.rs | 31 ++++++++++++++++++++++++-------
 src/db/models/organization.rs | 13 +++++++++++++
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs
index cbf029b5..8d9487e4 100644
--- a/src/api/core/organizations.rs
+++ b/src/api/core/organizations.rs
@@ -67,6 +67,7 @@ pub fn routes() -> Vec<Route> {
         import,
         post_org_keys,
         get_organization_keys,
+        get_organization_public_key,
         bulk_public_keys,
         deactivate_organization_user,
         bulk_deactivate_organization_user,
@@ -751,12 +752,19 @@ struct OrgIdData {
 }
 
 #[get("/ciphers/organization-details?<data..>")]
-async fn get_org_details(data: OrgIdData, headers: Headers, mut conn: DbConn) -> Json<Value> {
-    Json(json!({
+async fn get_org_details(data: OrgIdData, headers: Headers, mut conn: DbConn) -> JsonResult {
+    if UserOrganization::find_confirmed_by_user_and_org(&headers.user.uuid, &data.organization_id, &mut conn)
+        .await
+        .is_none()
+    {
+        err_code!("Resource not found.", rocket::http::Status::NotFound.code);
+    }
+
+    Ok(Json(json!({
         "data": _get_org_details(&data.organization_id, &headers.host, &headers.user.uuid, &mut conn).await,
         "object": "list",
         "continuationToken": null,
-    }))
+    })))
 }
 
 async fn _get_org_details(org_id: &str, host: &str, user_uuid: &str, conn: &mut DbConn) -> Value {
@@ -2748,20 +2756,29 @@ struct OrganizationUserResetPasswordRequest {
     key: String,
 }
 
-#[get("/organizations/<org_id>/keys")]
-async fn get_organization_keys(org_id: &str, mut conn: DbConn) -> JsonResult {
+// Upstrem reports this is the renamed endpoint instead of `/keys`
+// But the clients do not seem to use this at all
+// Just add it here in case they will
+#[get("/organizations/<org_id>/public-key")]
+async fn get_organization_public_key(org_id: &str, _headers: Headers, mut conn: DbConn) -> JsonResult {
     let org = match Organization::find_by_uuid(org_id, &mut conn).await {
         Some(organization) => organization,
         None => err!("Organization not found"),
     };
 
     Ok(Json(json!({
-        "object": "organizationKeys",
+        "object": "organizationPublicKey",
         "publicKey": org.public_key,
-        "privateKey": org.private_key,
     })))
 }
 
+// Obsolete - Renamed to public-key (2023.8), left for backwards compatibility with older clients
+// https://github.com/bitwarden/server/blob/25dc0c9178e3e3584074bbef0d4be827b7c89415/src/Api/AdminConsole/Controllers/OrganizationsController.cs#L463-L468
+#[get("/organizations/<org_id>/keys")]
+async fn get_organization_keys(org_id: &str, headers: Headers, conn: DbConn) -> JsonResult {
+    get_organization_public_key(org_id, headers, conn).await
+}
+
 #[put("/organizations/<org_id>/users/<org_user_id>/reset-password", data = "<data>")]
 async fn put_reset_password(
     org_id: &str,
diff --git a/src/db/models/organization.rs b/src/db/models/organization.rs
index 2d4d084b..ec390262 100644
--- a/src/db/models/organization.rs
+++ b/src/db/models/organization.rs
@@ -737,6 +737,19 @@ impl UserOrganization {
         }}
     }
 
+    pub async fn find_confirmed_by_user_and_org(user_uuid: &str, org_uuid: &str, conn: &mut DbConn) -> Option<Self> {
+        db_run! { conn: {
+            users_organizations::table
+                .filter(users_organizations::user_uuid.eq(user_uuid))
+                .filter(users_organizations::org_uuid.eq(org_uuid))
+                .filter(
+                    users_organizations::status.eq(UserOrgStatus::Confirmed as i32)
+                )
+                .first::<UserOrganizationDb>(conn)
+                .ok().from_db()
+        }}
+    }
+
     pub async fn find_by_user(user_uuid: &str, conn: &mut DbConn) -> Vec<Self> {
         db_run! { conn: {
             users_organizations::table