Merge branch 'fix/value-data-api-crash' into 'main'

fix: large value_data length in api crashes server

See merge request veilid/veilid!139
This commit is contained in:
Christien Rioux 2023-08-22 19:00:50 +00:00
commit 10ec693fb4
4 changed files with 51 additions and 11 deletions

View File

@ -25,7 +25,7 @@ pub fn decode_signed_value_data(
let signature = decode_signature512(&sr); let signature = decode_signature512(&sr);
Ok(SignedValueData::new( Ok(SignedValueData::new(
ValueData::new_with_seq(seq, data, writer), ValueData::new_with_seq(seq, data, writer).map_err(RPCError::protocol)?,
signature, signature,
)) ))
} }

View File

@ -402,9 +402,9 @@ impl StorageManager {
return Ok(None); return Ok(None);
} }
let seq = last_signed_value_data.value_data().seq(); let seq = last_signed_value_data.value_data().seq();
ValueData::new_with_seq(seq + 1, data, writer.key) ValueData::new_with_seq(seq + 1, data, writer.key)?
} else { } else {
ValueData::new(data, writer.key) ValueData::new(data, writer.key)?
}; };
// Validate with schema // Validate with schema

View File

@ -207,6 +207,6 @@ pub fn fix_veilidvaluechange() -> VeilidValueChange {
key: fix_typedkey(), key: fix_typedkey(),
subkeys: vec![1, 2, 3, 4], subkeys: vec![1, 2, 3, 4],
count: 5, count: 5,
value: ValueData::new_with_seq(23, b"ValueData".to_vec(), fix_cryptokey()), value: ValueData::new_with_seq(23, b"ValueData".to_vec(), fix_cryptokey()).unwrap(),
} }
} }

View File

@ -1,4 +1,5 @@
use super::*; use super::*;
use veilid_api::VeilidAPIResult;
#[derive(Clone, Default, PartialOrd, PartialEq, Eq, Ord, Serialize, Deserialize, JsonSchema)] #[derive(Clone, Default, PartialOrd, PartialEq, Eq, Ord, Serialize, Deserialize, JsonSchema)]
pub struct ValueData { pub struct ValueData {
@ -17,17 +18,25 @@ pub struct ValueData {
impl ValueData { impl ValueData {
pub const MAX_LEN: usize = 32768; pub const MAX_LEN: usize = 32768;
pub fn new(data: Vec<u8>, writer: PublicKey) -> Self { pub fn new(data: Vec<u8>, writer: PublicKey) -> VeilidAPIResult<Self> {
assert!(data.len() <= Self::MAX_LEN); if data.len() > Self::MAX_LEN {
Self { apibail_generic!("invalid size");
}
Ok(Self {
seq: 0, seq: 0,
data, data,
writer, writer,
} })
} }
pub fn new_with_seq(seq: ValueSeqNum, data: Vec<u8>, writer: PublicKey) -> Self { pub fn new_with_seq(
assert!(data.len() <= Self::MAX_LEN); seq: ValueSeqNum,
Self { seq, data, writer } data: Vec<u8>,
writer: PublicKey,
) -> VeilidAPIResult<Self> {
if data.len() > Self::MAX_LEN {
apibail_generic!("invalid size");
}
Ok(Self { seq, data, writer })
} }
pub fn seq(&self) -> ValueSeqNum { pub fn seq(&self) -> ValueSeqNum {
@ -56,3 +65,34 @@ impl fmt::Debug for ValueData {
.finish() .finish()
} }
} }
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn value_data_ok() {
assert!(ValueData::new(vec![0; ValueData::MAX_LEN], CryptoKey { bytes: [0; 32] }).is_ok());
assert!(ValueData::new_with_seq(
0,
vec![0; ValueData::MAX_LEN],
CryptoKey { bytes: [0; 32] }
)
.is_ok());
}
#[test]
fn value_data_too_long() {
assert!(ValueData::new(
vec![0; ValueData::MAX_LEN + 1],
CryptoKey { bytes: [0; 32] }
)
.is_err());
assert!(ValueData::new_with_seq(
0,
vec![0; ValueData::MAX_LEN + 1],
CryptoKey { bytes: [0; 32] }
)
.is_err());
}
}