diff options
author | Vika <vika@fireburn.ru> | 2022-07-10 20:16:54 +0300 |
---|---|---|
committer | Vika <vika@fireburn.ru> | 2022-07-10 20:16:54 +0300 |
commit | 41f3a6dfbb05ec2ce58dabcd5ef205137f7ee821 (patch) | |
tree | df9c682111f129f0acb312597396402fa813d480 | |
parent | 9ca0e358dc95e7358815886b061288f04a7d29af (diff) | |
download | kittybox-41f3a6dfbb05ec2ce58dabcd5ef205137f7ee821.tar.zst |
Security bugfix: fix Scopes::has_all() incorrectly checking scopes
Turns out it was comparing the list of required scopes with **itself**. Oops, that's a major security issue.
-rw-r--r-- | kittybox-rs/indieauth/src/scopes.rs | 17 |
1 files changed, 15 insertions, 2 deletions
diff --git a/kittybox-rs/indieauth/src/scopes.rs b/kittybox-rs/indieauth/src/scopes.rs index 6d1ed7e..db4778a 100644 --- a/kittybox-rs/indieauth/src/scopes.rs +++ b/kittybox-rs/indieauth/src/scopes.rs @@ -89,9 +89,12 @@ impl Scopes { } pub fn has_all(&self, scopes: &[Scope]) -> bool { scopes.iter() - .map(|s1| scopes.iter().any(|s2| s1 == s2)) + .map(|s1| self.iter().any(|s2| s1 == s2)) .all(|s| s) } + pub fn iter(&self) -> std::slice::Iter<'_, Scope> { + self.0.iter() + } } impl AsRef<[Scope]> for Scopes { fn as_ref(&self) -> &[Scope] { @@ -162,8 +165,18 @@ mod tests { let scope_str = scope_serialized.as_str().unwrap(); assert_eq!(scope_str, "create update delete media kittybox_internal_access"); - assert!(serde_json::from_value::<Scopes>(scope_serialized).unwrap().has_all(&scopes)) + assert!(serde_json::from_value::<Scopes>(scope_serialized).unwrap().has_all(&scopes)) + } + + #[test] + fn test_scope_has_all() { + let scopes = Scopes(vec![ + Scope::Create, Scope::Update, Scope::custom("draft") + ]); + + assert!(scopes.has_all(&[Scope::Create, Scope::custom("draft")])); + assert!(!scopes.has_all(&[Scope::Read, Scope::custom("kittybox_internal_access")])); } } |