From 41f3a6dfbb05ec2ce58dabcd5ef205137f7ee821 Mon Sep 17 00:00:00 2001 From: Vika Date: Sun, 10 Jul 2022 20:16:54 +0300 Subject: 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. --- kittybox-rs/indieauth/src/scopes.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'kittybox-rs/indieauth') 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::(scope_serialized).unwrap().has_all(&scopes)) + assert!(serde_json::from_value::(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")])); } } -- cgit 1.4.1