From 3d83f086a687ab6f88b2096b51eb375b22c486be Mon Sep 17 00:00:00 2001 From: Finn Bear Date: Fri, 2 Sep 2022 08:19:01 -0700 Subject: [PATCH] Fix unbounded resource usage in crypto and rand SQL functions (#94) --- lib/src/err/mod.rs | 2 +- lib/src/fnc/crypto.rs | 111 ++++++++++++++++++++++++++++++++++-------- lib/src/fnc/rand.rs | 37 +++++++++----- lib/src/fnc/string.rs | 10 +++- 4 files changed, 126 insertions(+), 34 deletions(-) diff --git a/lib/src/err/mod.rs b/lib/src/err/mod.rs index 2f5fa0b1..4537ff8d 100644 --- a/lib/src/err/mod.rs +++ b/lib/src/err/mod.rs @@ -77,7 +77,7 @@ pub enum Error { message: String, }, - /// The wrong number of arguments was given for the specified function + /// The wrong quantity or magnitude of arguments was given for the specified function #[error("Incorrect arguments for function {name}(). {message}")] InvalidArguments { name: String, diff --git a/lib/src/fnc/crypto.rs b/lib/src/fnc/crypto.rs index b6a1a14e..ad6cfd41 100644 --- a/lib/src/fnc/crypto.rs +++ b/lib/src/fnc/crypto.rs @@ -39,23 +39,72 @@ pub fn sha512(_: &Context, mut args: Vec) -> Result { Ok(val.into()) } +/// Allowed to cost this much more than default setting for each hash function. +const COST_ALLOWANCE: u32 = 4; + +/// Like verify_password, but takes a closure to determine whether the cost of performing the +/// operation is not too high. +macro_rules! bounded_verify_password { + ($algo: ident, $instance: expr, $password: expr, $hash: expr, $bound: expr) => { + if let (Some(salt), Some(expected_output)) = (&$hash.salt, &$hash.hash) { + if let Some(params) = + <$algo as PasswordHasher>::Params::try_from($hash).ok().filter($bound) + { + if let Ok(computed_hash) = $instance.hash_password_customized( + $password.as_ref(), + Some($hash.algorithm), + $hash.version, + params, + *salt, + ) { + if let Some(computed_output) = &computed_hash.hash { + expected_output == computed_output + } else { + false + } + } else { + false + } + } else { + false + } + } else { + false + } + }; + + ($algo: ident, $password: expr, $hash: expr, $bound: expr) => { + bounded_verify_password!($algo, $algo::default(), $password, $hash, $bound) + }; +} + pub mod argon2 { + use super::COST_ALLOWANCE; use crate::ctx::Context; use crate::err::Error; use crate::sql::value::Value; use argon2::{ - password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString}, + password_hash::{PasswordHash, PasswordHasher, SaltString}, Argon2, }; use rand::rngs::OsRng; - pub fn cmp(_: &Context, mut args: Vec) -> Result { - let algo = Argon2::default(); - let hash = args.remove(0).as_string(); - let pass = args.remove(0).as_string(); - let test = PasswordHash::new(&hash).unwrap(); - Ok(algo.verify_password(pass.as_ref(), &test).is_ok().into()) + pub fn cmp(_: &Context, args: Vec) -> Result { + let args: [Value; 2] = args.try_into().unwrap(); + let [hash, pass] = args.map(Value::as_string); + type Params<'a> = as PasswordHasher>::Params; + Ok(PasswordHash::new(&hash) + .ok() + .filter(|test| { + bounded_verify_password!(Argon2, pass, test, |params: &Params| { + params.m_cost() <= Params::DEFAULT_M_COST.saturating_mul(COST_ALLOWANCE) + && params.t_cost() <= Params::DEFAULT_T_COST.saturating_mul(COST_ALLOWANCE) + && params.p_cost() <= Params::DEFAULT_P_COST.saturating_mul(COST_ALLOWANCE) + }) + }) + .is_some() + .into()) } pub fn gen(_: &Context, mut args: Vec) -> Result { @@ -69,20 +118,33 @@ pub mod argon2 { pub mod pbkdf2 { + use super::COST_ALLOWANCE; use crate::ctx::Context; use crate::err::Error; use crate::sql::value::Value; use pbkdf2::{ - password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString}, + password_hash::{PasswordHash, PasswordHasher, SaltString}, Pbkdf2, }; use rand::rngs::OsRng; - pub fn cmp(_: &Context, mut args: Vec) -> Result { - let hash = args.remove(0).as_string(); - let pass = args.remove(0).as_string(); - let test = PasswordHash::new(&hash).unwrap(); - Ok(Pbkdf2.verify_password(pass.as_ref(), &test).is_ok().into()) + pub fn cmp(_: &Context, args: Vec) -> Result { + let args: [Value; 2] = args.try_into().unwrap(); + let [hash, pass] = args.map(Value::as_string); + type Params = ::Params; + Ok(PasswordHash::new(&hash) + .ok() + .filter(|test| { + bounded_verify_password!(Pbkdf2, Pbkdf2, pass, test, |params: &Params| { + params.rounds <= Params::default().rounds.saturating_mul(COST_ALLOWANCE) + && params.output_length + <= Params::default() + .output_length + .saturating_mul(COST_ALLOWANCE as usize) + }) + }) + .is_some() + .into()) } pub fn gen(_: &Context, mut args: Vec) -> Result { @@ -100,15 +162,26 @@ pub mod scrypt { use crate::sql::value::Value; use rand::rngs::OsRng; use scrypt::{ - password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString}, + password_hash::{PasswordHash, PasswordHasher, SaltString}, Scrypt, }; - pub fn cmp(_: &Context, mut args: Vec) -> Result { - let hash = args.remove(0).as_string(); - let pass = args.remove(0).as_string(); - let test = PasswordHash::new(&hash).unwrap(); - Ok(Scrypt.verify_password(pass.as_ref(), &test).is_ok().into()) + pub fn cmp(_: &Context, args: Vec) -> Result { + let args: [Value; 2] = args.try_into().unwrap(); + let [hash, pass] = args.map(Value::as_string); + type Params = ::Params; + Ok(PasswordHash::new(&hash) + .ok() + .filter(|test| { + bounded_verify_password!(Scrypt, Scrypt, pass, test, |params: &Params| { + // Scrypt is slow, use lower cost allowance. + params.log_n() <= Params::default().log_n().saturating_add(2) + && params.r() <= Params::default().r().saturating_mul(2) + && params.p() <= Params::default().p().saturating_mul(4) + }) + }) + .is_some() + .into()) } pub fn gen(_: &Context, mut args: Vec) -> Result { diff --git a/lib/src/fnc/rand.rs b/lib/src/fnc/rand.rs index 0a4144a8..2ea8808a 100644 --- a/lib/src/fnc/rand.rs +++ b/lib/src/fnc/rand.rs @@ -45,8 +45,17 @@ pub fn float(_: &Context, mut args: Vec) -> Result { pub fn guid(_: &Context, mut args: Vec) -> Result { match args.len() { 1 => { + // Only need 53 to uniquely identify all atoms in observable universe. + const LIMIT: usize = 64; let len = args.remove(0).as_int() as usize; - Ok(nanoid!(len, &ID_CHARS).into()) + if len > LIMIT { + Err(Error::InvalidArguments { + name: String::from("rand::guid"), + message: format!("The maximum length of a GUID is {}.", LIMIT), + }) + } else { + Ok(nanoid!(len, &ID_CHARS).into()) + } } 0 => Ok(nanoid!(20, &ID_CHARS).into()), _ => unreachable!(), @@ -68,33 +77,35 @@ pub fn int(_: &Context, mut args: Vec) -> Result { } pub fn string(_: &Context, mut args: Vec) -> Result { + // Limit how much time and bandwidth is spent. + const LIMIT: i64 = 2i64.pow(16); match args.len() { 2 => match args.remove(0).as_int() { - min if min >= 0 => match args.remove(0).as_int() { - max if max >= 0 && max < min => Ok(rand::thread_rng() - .sample_iter(&Alphanumeric) - .take(rand::thread_rng().gen_range(max as usize..=min as usize)) - .map(char::from) - .collect::() - .into()), - max if max >= 0 => Ok(rand::thread_rng() + min if (0..=LIMIT).contains(&min) => match args.remove(0).as_int() { + max if min <= max && max <= LIMIT => Ok(rand::thread_rng() .sample_iter(&Alphanumeric) .take(rand::thread_rng().gen_range(min as usize..=max as usize)) .map(char::from) .collect::() .into()), + max if max >= 0 && max <= min => Ok(rand::thread_rng() + .sample_iter(&Alphanumeric) + .take(rand::thread_rng().gen_range(max as usize..=min as usize)) + .map(char::from) + .collect::() + .into()), _ => Err(Error::InvalidArguments { name: String::from("rand::string"), - message: String::from("To generate a string of between X and Y characters in length, the 2 arguments must be positive numbers."), + message: format!("To generate a string of between X and Y characters in length, the 2 arguments must be positive numbers and no higher than {}.", LIMIT), }), }, _ => Err(Error::InvalidArguments { name: String::from("rand::string"), - message: String::from("To generate a string of between X and Y characters in length, the 2 arguments must be positive numbers."), + message: format!("To generate a string of between X and Y characters in length, the 2 arguments must be positive numbers and no higher than {}.", LIMIT), }), }, 1 => match args.remove(0).as_int() { - x if x >= 0 => Ok(rand::thread_rng() + x if (0..=LIMIT).contains(&x) => Ok(rand::thread_rng() .sample_iter(&Alphanumeric) .take(x as usize) .map(char::from) @@ -102,7 +113,7 @@ pub fn string(_: &Context, mut args: Vec) -> Result { .into()), _ => Err(Error::InvalidArguments { name: String::from("rand::string"), - message: String::from("To generate a string of X characters in length, the argument must be a positive number."), + message: format!("To generate a string of X characters in length, the argument must be a positive number and no higher than {}.", LIMIT), }), }, 0 => Ok(rand::thread_rng() diff --git a/lib/src/fnc/string.rs b/lib/src/fnc/string.rs index 28c7cf1b..6d57627f 100644 --- a/lib/src/fnc/string.rs +++ b/lib/src/fnc/string.rs @@ -36,7 +36,15 @@ pub fn repeat(_: &Context, args: Vec) -> Result { let [val_arg, num_arg]: [Value; 2] = args.try_into().unwrap(); let val = val_arg.as_string(); let num = num_arg.as_int() as usize; - Ok(val.repeat(num).into()) + const LIMIT: usize = 2usize.pow(20); + if val.len().saturating_mul(num) > LIMIT { + Err(Error::InvalidArguments { + name: String::from("string::repeat"), + message: format!("Output must not exceed {} bytes.", LIMIT), + }) + } else { + Ok(val.repeat(num).into()) + } } pub fn replace(_: &Context, args: Vec) -> Result {