Skip to content

Commit

Permalink
Fix sync with new native clients (dani-garcia#4932)
Browse files Browse the repository at this point in the history
  • Loading branch information
BlackDex authored Sep 9, 2024
1 parent 66baa5e commit dca1428
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/api/core/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
// Bitwarden does not process the import if there is one item invalid.
// Since we check for the size of the encrypted note length, we need to do that here to pre-validate it.
// TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks.
Cipher::validate_notes(&data.ciphers)?;
Cipher::validate_cipher_data(&data.ciphers)?;

let user_uuid = &headers.user.uuid;

Expand Down
4 changes: 2 additions & 2 deletions src/api/core/ciphers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ pub struct CipherData {
favorite: Option<bool>,
reprompt: Option<i32>,

password_history: Option<Value>,
pub password_history: Option<Value>,

// These are used during key rotation
// 'Attachments' is unused, contains map of {id: filename}
Expand Down Expand Up @@ -563,7 +563,7 @@ async fn post_ciphers_import(
// Bitwarden does not process the import if there is one item invalid.
// Since we check for the size of the encrypted note length, we need to do that here to pre-validate it.
// TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks.
Cipher::validate_notes(&data.ciphers)?;
Cipher::validate_cipher_data(&data.ciphers)?;

// Read and create the folders
let existing_folders: Vec<String> =
Expand Down
2 changes: 1 addition & 1 deletion src/api/core/organizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1596,7 +1596,7 @@ async fn post_org_import(
// Bitwarden does not process the import if there is one item invalid.
// Since we check for the size of the encrypted note length, we need to do that here to pre-validate it.
// TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks.
Cipher::validate_notes(&data.ciphers)?;
Cipher::validate_cipher_data(&data.ciphers)?;

let mut collections = Vec::new();
for coll in data.collections {
Expand Down
57 changes: 46 additions & 11 deletions src/db/models/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,39 @@ impl Cipher {
}
}

pub fn validate_notes(cipher_data: &[CipherData]) -> EmptyResult {
pub fn validate_cipher_data(cipher_data: &[CipherData]) -> EmptyResult {
let mut validation_errors = serde_json::Map::new();
let max_note_size = CONFIG._max_note_size();
let max_note_size_msg =
format!("The field Notes exceeds the maximum encrypted value length of {} characters.", &max_note_size);
for (index, cipher) in cipher_data.iter().enumerate() {
// Validate the note size and if it is exceeded return a warning
if let Some(note) = &cipher.notes {
if note.len() > max_note_size {
validation_errors
.insert(format!("Ciphers[{index}].Notes"), serde_json::to_value([&max_note_size_msg]).unwrap());
}
}

// Validate the password history if it contains `null` values and if so, return a warning
if let Some(Value::Array(password_history)) = &cipher.password_history {
for pwh in password_history {
if let Value::Object(pwo) = pwh {
if pwo.get("password").is_some_and(|p| !p.is_string()) {
validation_errors.insert(
format!("Ciphers[{index}].Notes"),
serde_json::to_value([
"The password history contains a `null` value. Only strings are allowed.",
])
.unwrap(),
);
break;
}
}
}
}
}

if !validation_errors.is_empty() {
let err_json = json!({
"message": "The model state is invalid.",
Expand Down Expand Up @@ -153,27 +173,39 @@ impl Cipher {
.as_ref()
.and_then(|s| {
serde_json::from_str::<Vec<LowerCase<Value>>>(s)
.inspect_err(|e| warn!("Error parsing fields {:?}", e))
.inspect_err(|e| warn!("Error parsing fields {e:?} for {}", self.uuid))
.ok()
})
.map(|d| d.into_iter().map(|d| d.data).collect())
.unwrap_or_default();

let password_history_json: Vec<_> = self
.password_history
.as_ref()
.and_then(|s| {
serde_json::from_str::<Vec<LowerCase<Value>>>(s)
.inspect_err(|e| warn!("Error parsing password history {:?}", e))
.inspect_err(|e| warn!("Error parsing password history {e:?} for {}", self.uuid))
.ok()
})
.map(|d| d.into_iter().map(|d| d.data).collect())
.map(|d| {
// Check every password history item if they are valid and return it.
// If a password field has the type `null` skip it, it breaks newer Bitwarden clients
d.into_iter()
.filter_map(|d| match d.data.get("password") {
Some(p) if p.is_string() => Some(d.data),
_ => None,
})
.collect()
})
.unwrap_or_default();

// Get the type_data or a default to an empty json object '{}'.
// If not passing an empty object, mobile clients will crash.
let mut type_data_json = serde_json::from_str::<LowerCase<Value>>(&self.data)
.map(|d| d.data)
.unwrap_or_else(|_| Value::Object(serde_json::Map::new()));
let mut type_data_json =
serde_json::from_str::<LowerCase<Value>>(&self.data).map(|d| d.data).unwrap_or_else(|_| {
warn!("Error parsing data field for {}", self.uuid);
Value::Object(serde_json::Map::new())
});

// NOTE: This was marked as *Backwards Compatibility Code*, but as of January 2021 this is still being used by upstream
// Set the first element of the Uris array as Uri, this is needed several (mobile) clients.
Expand All @@ -189,10 +221,13 @@ impl Cipher {

// Fix secure note issues when data is invalid
// This breaks at least the native mobile clients
if self.atype == 2
&& (self.data.is_empty() || self.data.eq("{}") || self.data.to_ascii_lowercase().eq("{\"type\":null}"))
{
type_data_json = json!({"type": 0});
if self.atype == 2 {
match type_data_json {
Value::Object(ref t) if t.get("type").is_some_and(|t| t.is_number()) => {}
_ => {
type_data_json = json!({"type": 0});
}
}
}

// Clone the type_data and add some default value.
Expand Down

0 comments on commit dca1428

Please sign in to comment.