[PR #5438] [MERGED] Security fixes #2585

New Issue

OVERLORD · 2025-10-09T18:07:45+03:00

OVERLORD commented

2025-10-09 18:07:45 +03:00

📋 Pull Request Information

Original PR: https://github.com/dani-garcia/vaultwarden/pull/5438
Author: @BlackDex
Created: 1/24/2025
Status: ✅ Merged
Merged: 1/25/2025
Merged by: @dani-garcia

Base: main ← Head: org-fixes

📝 Commits (7)

08fb368 Security fixes for admin and sendmail
fefae9f Fix security issue with organizationId validation
46fe50d Update server version in config endpoint
7c06c83 Fix and adjust build workflow
1f0f0cc Update crates
9f66551 Allow sendmail to be configurable
983c862 Add more org_id checks

📊 Changes

15 files changed (+418 additions, -223 deletions)

View changed files

📝 .github/workflows/build.yml (+8 -8)
📝 Cargo.lock (+84 -80)
📝 Cargo.toml (+7 -7)
📝 macros/Cargo.toml (+1 -1)
📝 src/api/admin.rs (+17 -17)
📝 src/api/core/events.rs (+9 -2)
📝 src/api/core/mod.rs (+2 -2)
📝 src/api/core/organizations.rs (+182 -61)
📝 src/auth.rs (+38 -14)
📝 src/config.rs (+32 -17)
📝 src/db/models/organization.rs (+3 -2)
📝 src/db/models/user.rs (+4 -4)
📝 src/mail.rs (+5 -6)
📝 src/static/scripts/admin_diagnostics.js (+4 -1)
📝 src/util.rs (+22 -1)

📄 Description

Security fixes for admin and sendmail

Because the Vaultwarden Admin Backend endpoints did not validated the Content-Type during a request, it was possible to update settings via CSRF. But, this was only possible if there was no ADMIN_TOKEN set at all. To make sure these environments are also safe I added the needed content-type checks at the functions.
This could cause some users who have scripts which uses cURL for example to adjust there commands to provide the correct headers.

By using a crafted favicon and having access to the Admin Backend an attacker could run custom commands on the host/container where Vaultwarden is running on. The main issue here is that we allowed the sendmail binary name/path to be updated via the Admin Backend. To mitigate this we removed this configuration item and only then sendmail binary as a name can be used.
This could cause some issues where the sendmail binary is not in the $PATH and thus not able to be started. In these cases the admins should make sure $PATH is set correctly or create a custom shell script or symlink at a location which is in the $PATH.

Added an extra security header and adjusted the CSP to be more strict by setting default-src to none and added the needed missing specific policies.

Also created a general email validation function which does some more checking to catch invalid email address not found by the email_address crate.

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 PR 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.

Update server version in config endpoint

Updated the server version reported to the clients to 2025.1.0.
This should make Vaultwarden future proof for the newer clients released by Bitwarden.

Fix and adjust build workflow

The build workflow had an issue with some if checks.
For one they had two $ signs, and it is not recommended to use always() since canceling a workflow does not cancel those calls.
Using !cancelled() is the preferred way.

Update crates

Updated crates to there latest version

Edit Allow sendmail binary config again.

Revert a previous change which removed the sendmail to be configurable.
We now set the config to be read-only, and omit all read-only values from being stored during a save action from the admin interface.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/dani-garcia/vaultwarden/pull/5438 **Author:** [@BlackDex](https://github.com/BlackDex) **Created:** 1/24/2025 **Status:** ✅ Merged **Merged:** 1/25/2025 **Merged by:** [@dani-garcia](https://github.com/dani-garcia) **Base:** `main` ← **Head:** `org-fixes` --- ### 📝 Commits (7) - [`08fb368`](https://github.com/dani-garcia/vaultwarden/commit/08fb368d5f8786be198d758dd5fe1df44402809a) Security fixes for admin and sendmail - [`fefae9f`](https://github.com/dani-garcia/vaultwarden/commit/fefae9fdee11c0cf1af4da128abd97e8025289f4) Fix security issue with organizationId validation - [`46fe50d`](https://github.com/dani-garcia/vaultwarden/commit/46fe50d9fbe66039c83b47a479f60fb24663f262) Update server version in config endpoint - [`7c06c83`](https://github.com/dani-garcia/vaultwarden/commit/7c06c83a258ed5435429e180e58a90b276da9343) Fix and adjust build workflow - [`1f0f0cc`](https://github.com/dani-garcia/vaultwarden/commit/1f0f0cc0ac22d08c9b93212a5c0930aa973f8983) Update crates - [`9f66551`](https://github.com/dani-garcia/vaultwarden/commit/9f66551289fea51b9473fe0503acba6193676921) Allow sendmail to be configurable - [`983c862`](https://github.com/dani-garcia/vaultwarden/commit/983c862d22249b301e760b074072cd01222aff0e) Add more org_id checks ### 📊 Changes **15 files changed** (+418 additions, -223 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/build.yml` (+8 -8) 📝 `Cargo.lock` (+84 -80) 📝 `Cargo.toml` (+7 -7) 📝 `macros/Cargo.toml` (+1 -1) 📝 `src/api/admin.rs` (+17 -17) 📝 `src/api/core/events.rs` (+9 -2) 📝 `src/api/core/mod.rs` (+2 -2) 📝 `src/api/core/organizations.rs` (+182 -61) 📝 `src/auth.rs` (+38 -14) 📝 `src/config.rs` (+32 -17) 📝 `src/db/models/organization.rs` (+3 -2) 📝 `src/db/models/user.rs` (+4 -4) 📝 `src/mail.rs` (+5 -6) 📝 `src/static/scripts/admin_diagnostics.js` (+4 -1) 📝 `src/util.rs` (+22 -1) </details> ### 📄 Description ### Security fixes for admin and sendmail Because the Vaultwarden Admin Backend endpoints did not validated the Content-Type during a request, it was possible to update settings via CSRF. But, this was only possible if there was no `ADMIN_TOKEN` set at all. To make sure these environments are also safe I added the needed content-type checks at the functions. This could cause some users who have scripts which uses cURL for example to adjust there commands to provide the correct headers. By using a crafted favicon and having access to the Admin Backend an attacker could run custom commands on the host/container where Vaultwarden is running on. The main issue here is that we allowed the sendmail binary name/path to be updated via the Admin Backend. ~~To mitigate this we removed this configuration item and only then `sendmail` binary as a name can be used. This could cause some issues where the `sendmail` binary is not in the `$PATH` and thus not able to be started. In these cases the admins should make sure `$PATH` is set correctly or create a custom shell script or symlink at a location which is in the `$PATH`.~~ Added an extra security header and adjusted the CSP to be more strict by setting `default-src` to `none` and added the needed missing specific policies. Also created a general email validation function which does some more checking to catch invalid email address not found by the email_address crate. ### 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 PR 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. ### Update server version in config endpoint Updated the server version reported to the clients to `2025.1.0`. This should make Vaultwarden future proof for the newer clients released by Bitwarden. ### Fix and adjust build workflow The build workflow had an issue with some `if` checks. For one they had two `$` signs, and it is not recommended to use `always()` since canceling a workflow does not cancel those calls. Using `!cancelled()` is the preferred way. ### Update crates Updated crates to there latest version --- ### Edit Allow sendmail binary config again. Revert a previous change which removed the sendmail to be configurable. We now set the config to be read-only, and omit all read-only values from being stored during a save action from the admin interface. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

OVERLORD added the pull-request label 2025-10-09 18:07:45 +03:00

OVERLORD closed this issue

2025-10-09 18:07:46 +03:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/vaultwarden#2585