Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion crates/bitwarden-vault/src/cipher/attachment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ pub struct AttachmentView {
/// Do not rely on this field for long-term use.
#[cfg(feature = "wasm")]
pub decrypted_key: Option<String>,
/// Indicates whether this attachment failed to decrypt.
#[serde(default)]
pub decryption_failure: bool,
}

impl AttachmentView {
Expand Down Expand Up @@ -211,6 +214,12 @@ impl Decryptable<KeyIds, SymmetricKeyId, AttachmentView> for Attachment {
ctx: &mut KeyStoreContext<KeyIds>,
key: SymmetricKeyId,
) -> Result<AttachmentView, CryptoError> {
// Try to decrypt the file name
let (file_name, decryption_failure) = match self.file_name.decrypt(ctx, key) {
Ok(name) => (name, false),
Err(..) => (None, true),
};
Comment on lines +218 to +221
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the SDK swallows any decryption errors for Attachments and thus hides all attachments from the user.

Where is the actual error being swallowed? I would suggest adding the handling logic there, maybe following the pattern of decrypt_list_with_failures. with the change here, you end up throwing away the CryptoError and the function returns as a "successful" decryption, breaking the contract of "This function returns a decrypted object, or an Err(CryptoError)"


#[cfg(feature = "wasm")]
let decrypted_key = if let Some(attachment_key) = &self.key {
let content_key_id = ctx.unwrap_symmetric_key(key, attachment_key)?;
Expand All @@ -228,7 +237,8 @@ impl Decryptable<KeyIds, SymmetricKeyId, AttachmentView> for Attachment {
url: self.url.clone(),
size: self.size.clone(),
size_name: self.size_name.clone(),
file_name: self.file_name.decrypt(ctx, key)?,
file_name,
decryption_failure,
key: self.key.clone(),
#[cfg(feature = "wasm")]
decrypted_key: decrypted_key.map(|k| k.to_string()),
Expand Down Expand Up @@ -290,6 +300,7 @@ mod tests {
key: None,
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
};

let contents = b"This is a test file that we will encrypt. It's 100 bytes long, the encrypted version will be longer!";
Expand Down Expand Up @@ -350,6 +361,7 @@ mod tests {
key: Some("2.r288/AOSPiaLFkW07EBGBw==|SAmnnCbOLFjX5lnURvoualOetQwuyPc54PAmHDTRrhT0gwO9ailna9U09q9bmBfI5XrjNNEsuXssgzNygRkezoVQvZQggZddOwHB6KQW5EQ=|erIMUJp8j+aTcmhdE50zEX+ipv/eR1sZ7EwULJm/6DY=".parse().unwrap()),
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
};

let cipher = Cipher {
Expand Down Expand Up @@ -411,6 +423,7 @@ mod tests {
key: None,
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
};

let cipher = Cipher {
Expand Down
4 changes: 4 additions & 0 deletions crates/bitwarden-vault/src/cipher/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,7 @@ mod tests {
key: None,
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
};
cipher.attachments = Some(vec![attachment]);

Expand Down Expand Up @@ -1567,6 +1568,7 @@ mod tests {
key: None,
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
};
cipher.attachments = Some(vec![attachment]);

Expand Down Expand Up @@ -1612,6 +1614,7 @@ mod tests {
key: Some(attachment_key_enc),
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
};
cipher.attachments = Some(vec![attachment]);
let cred = generate_fido2(&mut key_store.context(), SymmetricKeyId::User);
Expand Down Expand Up @@ -1681,6 +1684,7 @@ mod tests {
key: Some(attachment_key_enc.clone()),
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
};
cipher.attachments = Some(vec![attachment]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ mod tests {
key: None, // No key!
#[cfg(feature = "wasm")]
decrypted_key: None,
decryption_failure: false,
}]);

let organization_id: OrganizationId = TEST_ORG_ID.parse().unwrap();
Expand Down
Loading