diff --git a/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows/crypto.rs b/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows/crypto.rs index 9b91746dd1..094dbf94a6 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows/crypto.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows/crypto.rs @@ -2,17 +2,14 @@ use aes_gcm::{aead::Aead, Aes256Gcm, Key, KeyInit}; use anyhow::{anyhow, Result}; use base64::{engine::general_purpose, Engine as _}; use chacha20poly1305::ChaCha20Poly1305; +use chromium_importer::chromium::crypt_unprotect_data; use scopeguard::defer; use tracing::debug; use windows::{ core::w, - Win32::{ - Foundation::{LocalFree, HLOCAL}, - Security::Cryptography::{ - self, CryptUnprotectData, NCryptOpenKey, NCryptOpenStorageProvider, CERT_KEY_SPEC, - CRYPTPROTECT_UI_FORBIDDEN, CRYPT_INTEGER_BLOB, NCRYPT_FLAGS, NCRYPT_KEY_HANDLE, - NCRYPT_PROV_HANDLE, NCRYPT_SILENT_FLAG, - }, + Win32::Security::Cryptography::{ + self, NCryptOpenKey, NCryptOpenStorageProvider, CERT_KEY_SPEC, CRYPTPROTECT_UI_FORBIDDEN, + NCRYPT_FLAGS, NCRYPT_KEY_HANDLE, NCRYPT_PROV_HANDLE, NCRYPT_SILENT_FLAG, }, }; @@ -71,38 +68,7 @@ fn decrypt_with_dpapi(data: &[u8], expect_appb: bool) -> Result> { let data = if expect_appb { &data[4..] } else { data }; - let in_blob = CRYPT_INTEGER_BLOB { - cbData: data.len() as u32, - pbData: data.as_ptr() as *mut u8, - }; - - let mut out_blob = CRYPT_INTEGER_BLOB::default(); - - let result = unsafe { - CryptUnprotectData( - &in_blob, - None, - None, - None, - None, - CRYPTPROTECT_UI_FORBIDDEN, - &mut out_blob, - ) - }; - - if result.is_ok() && !out_blob.pbData.is_null() && out_blob.cbData > 0 { - let decrypted = unsafe { - std::slice::from_raw_parts(out_blob.pbData, out_blob.cbData as usize).to_vec() - }; - - // Free the memory allocated by CryptUnprotectData - unsafe { LocalFree(Some(HLOCAL(out_blob.pbData as *mut _))) }; - - Ok(decrypted) - } else { - debug!("CryptUnprotectData failed"); - Err(anyhow!("CryptUnprotectData failed")) - } + crypt_unprotect_data(data, CRYPTPROTECT_UI_FORBIDDEN) } // diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs index c6bbd3af44..aec8a84b5c 100644 --- a/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs @@ -10,7 +10,7 @@ use rusqlite::{params, Connection}; mod platform; #[cfg(target_os = "windows")] -pub use platform::{verify_signature, ADMIN_TO_USER_PIPE_NAME}; +pub use platform::*; pub(crate) use platform::SUPPORTED_BROWSERS as PLATFORM_SUPPORTED_BROWSERS; diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/crypto.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/crypto.rs new file mode 100644 index 0000000000..60f7b80603 --- /dev/null +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/crypto.rs @@ -0,0 +1,54 @@ +use anyhow::{anyhow, Result}; +use windows::Win32::{ + Foundation::{LocalFree, HLOCAL}, + Security::Cryptography::{CryptUnprotectData, CRYPT_INTEGER_BLOB}, +}; + +/// Rust friendly wrapper around CryptUnprotectData +/// +/// Decrypts the data passed in using the `CryptUnprotectData` api. +pub fn crypt_unprotect_data(data: &[u8], flags: u32) -> Result> { + if data.is_empty() { + return Ok(Vec::new()); + } + + let data_in = CRYPT_INTEGER_BLOB { + cbData: data.len() as u32, + pbData: data.as_ptr() as *mut u8, + }; + + let mut data_out = CRYPT_INTEGER_BLOB::default(); + + let result = unsafe { + CryptUnprotectData( + &data_in, + None, // ppszDataDescr: Option<*mut PWSTR> + None, // pOptionalEntropy: Option<*const CRYPT_INTEGER_BLOB> + None, // pvReserved: Option<*const std::ffi::c_void> + None, // pPromptStruct: Option<*const CRYPTPROTECT_PROMPTSTRUCT> + flags, // dwFlags: u32 + &mut data_out, + ) + }; + + if result.is_err() { + return Err(anyhow!("CryptUnprotectData failed")); + } + + if data_out.pbData.is_null() || data_out.cbData == 0 { + return Ok(Vec::new()); + } + + let output_slice = + unsafe { std::slice::from_raw_parts(data_out.pbData, data_out.cbData as usize) }; + + // SAFETY: Must copy data before calling LocalFree() below. + // Calling to_vec() after LocalFree() causes use-after-free bugs. + let output = output_slice.to_vec(); + + unsafe { + LocalFree(Some(HLOCAL(data_out.pbData as *mut _))); + } + + Ok(output) +} diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs index a1191f2eba..867104d9bf 100644 --- a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs @@ -3,18 +3,16 @@ use anyhow::{anyhow, Result}; use async_trait::async_trait; use base64::{engine::general_purpose::STANDARD as BASE64_STANDARD, Engine as _}; use std::path::{Path, PathBuf}; -use windows::Win32::{ - Foundation::{LocalFree, HLOCAL}, - Security::Cryptography::{CryptUnprotectData, CRYPT_INTEGER_BLOB}, -}; use crate::chromium::{BrowserConfig, CryptoService, LocalState}; use crate::util; mod abe; mod abe_config; +mod crypto; mod signature; pub use abe_config::ADMIN_TO_USER_PIPE_NAME; +pub use crypto::*; pub use signature::*; // @@ -166,7 +164,7 @@ impl WindowsCryptoService { return Err(anyhow!("Encrypted master key is not encrypted with DPAPI")); } - let key = unprotect_data_win(&key_bytes[5..]) + let key = crypt_unprotect_data(&key_bytes[5..], 0) .map_err(|e| anyhow!("Failed to unprotect the master key: {}", e))?; Ok(key) @@ -209,53 +207,6 @@ impl WindowsCryptoService { } } -fn unprotect_data_win(data: &[u8]) -> Result> { - if data.is_empty() { - return Ok(Vec::new()); - } - - let data_in = CRYPT_INTEGER_BLOB { - cbData: data.len() as u32, - pbData: data.as_ptr() as *mut u8, - }; - - let mut data_out = CRYPT_INTEGER_BLOB { - cbData: 0, - pbData: std::ptr::null_mut(), - }; - - let result = unsafe { - CryptUnprotectData( - &data_in, - None, // ppszDataDescr: Option<*mut PWSTR> - None, // pOptionalEntropy: Option<*const CRYPT_INTEGER_BLOB> - None, // pvReserved: Option<*const std::ffi::c_void> - None, // pPromptStruct: Option<*const CRYPTPROTECT_PROMPTSTRUCT> - 0, // dwFlags: u32 - &mut data_out, - ) - }; - - if result.is_err() { - return Err(anyhow!("CryptUnprotectData failed")); - } - - if data_out.pbData.is_null() || data_out.cbData == 0 { - return Ok(Vec::new()); - } - - let output_slice = - unsafe { std::slice::from_raw_parts(data_out.pbData, data_out.cbData as usize) }; - - unsafe { - if !data_out.pbData.is_null() { - LocalFree(Some(HLOCAL(data_out.pbData as *mut std::ffi::c_void))); - } - } - - Ok(output_slice.to_vec()) -} - fn get_admin_exe_path() -> Result { let current_exe_full_path = std::env::current_exe() .map_err(|e| anyhow!("Failed to get current executable path: {}", e))?;