From 094a1896a77c17780f30288dbeb27b68e026d627 Mon Sep 17 00:00:00 2001 From: Jeremy Lin Date: Fri, 29 Oct 2021 11:45:43 -0700 Subject: [PATCH] Fix conflict resolution logic for `read_only` and `hide_passwords` flags For one of these flags to be in effect for a cipher, upstream requires all of (rather than any of) the collections the cipher is in to have that flag set. Also, some of the logic for loading access restrictions was wrong. I think that only malicious clients that also had knowledge of the UUIDs of ciphers they didn't have access to would have been able to take advantage of that. --- src/db/models/cipher.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index 437934c79e..834fc6d332 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -355,17 +355,19 @@ impl Cipher { // There's an edge case where a cipher can be in multiple collections // with inconsistent access flags. For example, a cipher could be in // one collection where the user has read-only access, but also in - // another collection where the user has read/write access. To handle - // this, we do a boolean OR of all values in each of the `read_only` - // and `hide_passwords` columns. This could ideally be done as part - // of the query, but Diesel doesn't support a max() or bool_or() - // function on booleans and this behavior isn't portable anyway. + // another collection where the user has read/write access. For a flag + // to be in effect for a cipher, upstream requires all collections the + // cipher is in to have that flag set. Therefore, we do a boolean AND + // of all values in each of the `read_only` and `hide_passwords` columns. + // This could ideally be done as part of the query, but Diesel doesn't + // support a min() or bool_and() function on booleans and this behavior + // isn't portable anyway. if let Ok(vec) = query.load::<(bool, bool)>(conn) { - let mut read_only = false; - let mut hide_passwords = false; + let mut read_only = true; + let mut hide_passwords = true; for (ro, hp) in vec.iter() { - read_only |= ro; - hide_passwords |= hp; + read_only &= ro; + hide_passwords &= hp; } Some((read_only, hide_passwords))