[PM-7067] Remove unnecessary unwraps (#682)

## Type of change ``` - [ ] Bug fix - [ ] New feature development - [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ``` ## Objective This PR replaces the unwraps found in the audit by proper error handling, I've also had a quick look through the rest of the code base and replaced a bunch of others as well.
bitwarden · Mar 28, 2024 · 51a5140 · 51a5140
1 parent 27cf054
commit 51a5140
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 27 deletions.
diff --git a/crates/bitwarden-crypto/src/keys/master_key.rs b/crates/bitwarden-crypto/src/keys/master_key.rs
@@ -5,7 +5,7 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
 use super::utils::{derive_kdf_key, stretch_kdf_key};
-use crate::{util, EncString, KeyDecryptable, Result, SymmetricCryptoKey, UserKey};
+use crate::{util, CryptoError, EncString, KeyDecryptable, Result, SymmetricCryptoKey, UserKey};
 
 #[derive(Serialize, Deserialize, Debug, JsonSchema, Clone)]
 #[serde(rename_all = "camelCase", deny_unknown_fields)]
@@ -68,7 +68,10 @@ impl MasterKey {
 
         EncString::encrypt_aes256_hmac(
             user_key.to_vec().as_slice(),
-            stretched_key.mac_key.as_ref().unwrap(),
+            stretched_key
+                .mac_key
+                .as_ref()
+                .ok_or(CryptoError::InvalidMac)?,
             &stretched_key.key,
         )
     }

diff --git a/crates/bitwarden-crypto/src/rsa.rs b/crates/bitwarden-crypto/src/rsa.rs
@@ -36,8 +36,11 @@ pub(crate) fn make_key_pair(key: &SymmetricCryptoKey) -> Result<RsaKeyPair> {
         .to_pkcs8_der()
         .map_err(|_| RsaError::CreatePrivateKey)?;
 
-    let protected =
-        EncString::encrypt_aes256_hmac(pkcs.as_bytes(), key.mac_key.as_ref().unwrap(), &key.key)?;
+    let protected = EncString::encrypt_aes256_hmac(
+        pkcs.as_bytes(),
+        key.mac_key.as_ref().ok_or(CryptoError::InvalidMac)?,
+        &key.key,
+    )?;
 
     Ok(RsaKeyPair {
         public: b64,

diff --git a/crates/bitwarden/src/auth/login/api_key.rs b/crates/bitwarden/src/auth/login/api_key.rs
@@ -9,7 +9,7 @@ use crate::{
         JWTToken,
     },
     client::{LoginMethod, UserLoginMethod},
-    error::Result,
+    error::{Error, Result},
     Client,
 };
 
@@ -44,8 +44,12 @@ pub(crate) async fn login_api_key(
             kdf,
         }));
 
-        let user_key: EncString = r.key.as_deref().unwrap().parse().unwrap();
-        let private_key: EncString = r.private_key.as_deref().unwrap().parse().unwrap();
+        let user_key: EncString = r.key.as_deref().ok_or(Error::MissingFields)?.parse()?;
+        let private_key: EncString = r
+            .private_key
+            .as_deref()
+            .ok_or(Error::MissingFields)?
+            .parse()?;
 
         client.initialize_user_crypto(&input.password, user_key, private_key)?;
     }

diff --git a/crates/bitwarden/src/auth/login/auth_request.rs b/crates/bitwarden/src/auth/login/auth_request.rs
@@ -13,7 +13,7 @@ use crate::{
         auth_request::new_auth_request,
     },
     client::{LoginMethod, UserLoginMethod},
-    error::Result,
+    error::{Error, Result},
     mobile::crypto::{AuthRequestMethod, InitUserCryptoMethod, InitUserCryptoRequest},
     Client,
 };
@@ -50,7 +50,7 @@ pub(crate) async fn send_new_auth_request(
         fingerprint: auth.fingerprint,
         email,
         device_identifier,
-        auth_request_id: res.id.unwrap(),
+        auth_request_id: res.id.ok_or(Error::MissingFields)?,
         access_code: auth.access_code,
         private_key: auth.private_key,
     })
@@ -103,11 +103,11 @@ pub(crate) async fn complete_auth_request(
 
         let method = match res.master_password_hash {
             Some(_) => AuthRequestMethod::MasterKey {
-                protected_master_key: res.key.unwrap().parse().unwrap(),
-                auth_request_key: r.key.unwrap().parse().unwrap(),
+                protected_master_key: res.key.ok_or(Error::MissingFields)?.parse()?,
+                auth_request_key: r.key.ok_or(Error::MissingFields)?.parse()?,
             },
             None => AuthRequestMethod::UserKey {
-                protected_user_key: res.key.unwrap().parse().unwrap(),
+                protected_user_key: res.key.ok_or(Error::MissingFields)?.parse()?,
             },
         };
 
@@ -116,7 +116,7 @@ pub(crate) async fn complete_auth_request(
             .initialize_user_crypto(InitUserCryptoRequest {
                 kdf_params: kdf,
                 email: auth_req.email,
-                private_key: r.private_key.unwrap(),
+                private_key: r.private_key.ok_or(Error::MissingFields)?,
                 method: InitUserCryptoMethod::AuthRequest {
                     request_private_key: auth_req.private_key,
                     method,

diff --git a/crates/bitwarden/src/auth/login/password.rs b/crates/bitwarden/src/auth/login/password.rs
@@ -24,7 +24,7 @@ pub(crate) async fn login_password(
 ) -> Result<PasswordLoginResponse> {
     use bitwarden_crypto::{EncString, HashPurpose};
 
-    use crate::{auth::determine_password_hash, client::UserLoginMethod};
+    use crate::{auth::determine_password_hash, client::UserLoginMethod, error::Error};
 
     info!("password logging in");
     debug!("{:#?}, {:#?}", client, input);
@@ -49,8 +49,12 @@ pub(crate) async fn login_password(
             kdf: input.kdf.to_owned(),
         }));
 
-        let user_key: EncString = r.key.as_deref().unwrap().parse().unwrap();
-        let private_key: EncString = r.private_key.as_deref().unwrap().parse().unwrap();
+        let user_key: EncString = r.key.as_deref().ok_or(Error::MissingFields)?.parse()?;
+        let private_key: EncString = r
+            .private_key
+            .as_deref()
+            .ok_or(Error::MissingFields)?
+            .parse()?;
 
         client.initialize_user_crypto(&input.password, user_key, private_key)?;
     }

diff --git a/crates/bitwarden/src/mobile/vault/client_attachments.rs b/crates/bitwarden/src/mobile/vault/client_attachments.rs
@@ -40,7 +40,7 @@ impl<'a> ClientAttachments<'a> {
         decrypted_file_path: &Path,
         encrypted_file_path: &Path,
     ) -> Result<Attachment> {
-        let data = std::fs::read(decrypted_file_path).unwrap();
+        let data = std::fs::read(decrypted_file_path)?;
         let AttachmentEncryptResult {
             attachment,
             contents,
@@ -73,7 +73,7 @@ impl<'a> ClientAttachments<'a> {
         encrypted_file_path: &Path,
         decrypted_file_path: &Path,
     ) -> Result<()> {
-        let data = std::fs::read(encrypted_file_path).unwrap();
+        let data = std::fs::read(encrypted_file_path)?;
         let decrypted = self.decrypt_buffer(cipher, attachment, &data).await?;
         std::fs::write(decrypted_file_path, decrypted)?;
         Ok(())

diff --git a/crates/bitwarden/src/mobile/vault/client_sends.rs b/crates/bitwarden/src/mobile/vault/client_sends.rs
@@ -36,7 +36,7 @@ impl<'a> ClientSends<'a> {
         encrypted_file_path: &Path,
         decrypted_file_path: &Path,
     ) -> Result<()> {
-        let data = std::fs::read(encrypted_file_path).unwrap();
+        let data = std::fs::read(encrypted_file_path)?;
         let decrypted = self.decrypt_buffer(send, &data).await?;
         std::fs::write(decrypted_file_path, decrypted)?;
         Ok(())
@@ -65,7 +65,7 @@ impl<'a> ClientSends<'a> {
         decrypted_file_path: &Path,
         encrypted_file_path: &Path,
     ) -> Result<()> {
-        let data = std::fs::read(decrypted_file_path).unwrap();
+        let data = std::fs::read(decrypted_file_path)?;
         let encrypted = self.encrypt_buffer(send, &data).await?;
         std::fs::write(encrypted_file_path, encrypted)?;
         Ok(())

diff --git a/crates/bitwarden/src/vault/cipher/login.rs b/crates/bitwarden/src/vault/cipher/login.rs
@@ -238,7 +238,7 @@ impl TryFrom<bitwarden_api_api::models::CipherFido2CredentialModel> for Fido2Cre
                 .ok()
                 .flatten(),
             discoverable: value.discoverable.ok_or(Error::MissingFields)?.parse()?,
-            creation_date: value.creation_date.parse().unwrap(),
+            creation_date: value.creation_date.parse()?,
         })
     }
 }

diff --git a/crates/bw/src/auth/login.rs b/crates/bw/src/auth/login.rs
@@ -123,17 +123,13 @@ pub(crate) async fn login_device(
     let email = text_prompt_when_none("Email", email)?;
     let device_identifier = text_prompt_when_none("Device Identifier", device_identifier)?;
 
-    let auth = client
-        .auth()
-        .login_device(email, device_identifier)
-        .await
-        .unwrap();
+    let auth = client.auth().login_device(email, device_identifier).await?;
 
     println!("Fingerprint: {}", auth.fingerprint);
 
     Text::new("Press enter once approved").prompt()?;
 
-    client.auth().login_device_complete(auth).await.unwrap();
+    client.auth().login_device_complete(auth).await?;
 
     Ok(())
 }