Skip to content

Commit

Permalink
Fix data disclosure on organization endpoints (#4837)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
BlackDex committed Aug 11, 2024
1 parent a1204cc commit b557c11
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 7 deletions.
31 changes: 24 additions & 7 deletions src/api/core/organizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions src/db/models/organization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b557c11

Please sign in to comment.