Fix security issue with organizationId validation

Because of a invalid check/validation of the OrganizationId which most of the time is located in the path but sometimes provided as a URL Parameter, the parameter overruled the path ID during the Guard checks.
This resulted in someone being able to execute commands as an Admin or Owner of the OrganizationId fetched from the parameter, but the API endpoints then used the OrganizationId located in the path instead.

This commit fixes the extraction of the OrganizationId in the Guard and also added some extra validations of this OrgId in several functions.

Also added an extra `OrgMemberHeaders` which can be used to only allow access to organization endpoints which should only be accessible by members of that org.

Signed-off-by: BlackDex <black.dex@gmail.com>

src/api/core/organizations.rs

@@ -10,7 +10,10 @@ use crate::{
         core::{log_event, two_factor, CipherSyncData, CipherSyncType},
         EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType,
     },
-    auth::{decode_invite, AdminHeaders, ClientVersion, Headers, ManagerHeaders, ManagerHeadersLoose, OwnerHeaders},
+    auth::{
+        decode_invite, AdminHeaders, ClientVersion, Headers, ManagerHeaders, ManagerHeadersLoose, OrgMemberHeaders,
+        OwnerHeaders,
+    },
     db::{models::*, DbConn},
     mail,
     util::{convert_json_key_lcase_first, NumberOrString},
@@ -213,6 +216,9 @@ async fn delete_organization(
     headers: OwnerHeaders,
     mut conn: DbConn,
 ) -> EmptyResult {
+    if org_id != headers.org_id {
+        err!("Organization not found", "Organization id's do not match");
+    }
     let data: PasswordOrOtpData = data.into_inner();
 
     data.validate(&headers.user, true, &mut conn).await?;
@@ -261,7 +267,10 @@ async fn leave_organization(org_id: OrganizationId, headers: Headers, mut conn:
 }
 
 #[get("/organizations/<org_id>")]
-async fn get_organization(org_id: OrganizationId, _headers: OwnerHeaders, mut conn: DbConn) -> JsonResult {
+async fn get_organization(org_id: OrganizationId, headers: OwnerHeaders, mut conn: DbConn) -> JsonResult {
+    if org_id != headers.org_id {
+        err!("Organization not found", "Organization id's do not match");
+    }
     match Organization::find_by_uuid(&org_id, &mut conn).await {
         Some(organization) => Ok(Json(organization.to_json())),
         None => err!("Can't find organization details"),
@@ -285,14 +294,18 @@ async fn post_organization(
     data: Json<OrganizationUpdateData>,
     mut conn: DbConn,
 ) -> JsonResult {
+    if org_id != headers.org_id {
+        err!("Organization not found", "Organization id's do not match");
+    }
+
     let data: OrganizationUpdateData = data.into_inner();
 
     let Some(mut org) = Organization::find_by_uuid(&org_id, &mut conn).await else {
-        err!("Can't find organization details")
+        err!("Organization not found")
     };
 
     org.name = data.name;
-    org.billing_email = data.billing_email;
+    org.billing_email = data.billing_email.to_lowercase();
 
     org.save(&mut conn).await?;
 
@@ -778,10 +791,9 @@ struct OrgIdData {
 }
 
 #[get("/ciphers/organization-details?<data..>")]
-async fn get_org_details(data: OrgIdData, headers: Headers, mut conn: DbConn) -> JsonResult {
+async fn get_org_details(data: OrgIdData, headers: OrgMemberHeaders, mut conn: DbConn) -> JsonResult {
-    if Membership::find_confirmed_by_user_and_org(&headers.user.uuid, &data.organization_id, &mut conn).await.is_none()
+    if data.organization_id != headers.org_id {
-    {
+        err_code!("Resource not found.", "Organization id's do not match", rocket::http::Status::NotFound.code);
-        err_code!("Resource not found.", rocket::http::Status::NotFound.code);
     }
 
     Ok(Json(json!({

src/auth.rs

@@ -557,24 +557,17 @@ impl<'r> FromRequest<'r> for OrgHeaders {
         // but there are cases where it is a query value.
         // First check the path, if this is not a valid uuid, try the query values.
         let url_org_id: Option<OrganizationId> = {
-            let mut url_org_id = None;
+            if let Some(Ok(org_id)) = request.param::<OrganizationId>(1) {
-            if let Some(Ok(org_id)) = request.param::<&str>(1) {
+                Some(org_id.clone())
-                if uuid::Uuid::parse_str(org_id).is_ok() {
+            } else if let Some(Ok(org_id)) = request.query_value::<OrganizationId>("organizationId") {
-                    url_org_id = Some(org_id.to_string().into());
+                Some(org_id.clone())
-                }
+            } else {
-            }
+                None
-
-            if let Some(Ok(org_id)) = request.query_value::<&str>("organizationId") {
-                if uuid::Uuid::parse_str(org_id).is_ok() {
-                    url_org_id = Some(org_id.to_string().into());
-                }
             }
-
-            url_org_id
         };
 
         match url_org_id {
-            Some(org_id) => {
+            Some(org_id) if uuid::Uuid::parse_str(&org_id).is_ok() => {
                 let mut conn = match DbConn::from_request(request).await {
                     Outcome::Success(conn) => conn,
                     _ => err_handler!("Error getting DB"),
@@ -794,6 +787,7 @@ pub struct OwnerHeaders {
     pub device: Device,
     pub user: User,
     pub ip: ClientIp,
+    pub org_id: OrganizationId,
 }
 
 #[rocket::async_trait]
@@ -807,6 +801,7 @@ impl<'r> FromRequest<'r> for OwnerHeaders {
                 device: headers.device,
                 user: headers.user,
                 ip: headers.ip,
+                org_id: headers.membership.org_uuid,
             })
         } else {
             err_handler!("You need to be Owner to call this endpoint")
@@ -814,6 +809,30 @@ impl<'r> FromRequest<'r> for OwnerHeaders {
     }
 }
 
+pub struct OrgMemberHeaders {
+    pub host: String,
+    pub user: User,
+    pub org_id: OrganizationId,
+}
+
+#[rocket::async_trait]
+impl<'r> FromRequest<'r> for OrgMemberHeaders {
+    type Error = &'static str;
+
+    async fn from_request(request: &'r Request<'_>) -> Outcome<Self, Self::Error> {
+        let headers = try_outcome!(OrgHeaders::from_request(request).await);
+        if headers.membership_type >= MembershipType::User {
+            Outcome::Success(Self {
+                host: headers.host,
+                user: headers.user,
+                org_id: headers.membership.org_uuid,
+            })
+        } else {
+            err_handler!("You need to be a Member of the Organization to call this endpoint")
+        }
+    }
+}
+
 //
 // Client IP address detection
 //