Skip to content

Commit

Permalink
fix(rust): account for minicbor length calculation bug
Browse files Browse the repository at this point in the history
  • Loading branch information
SanjoDeundiak committed Jun 26, 2024
1 parent d389ad6 commit 64462e1
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 17 deletions.
13 changes: 7 additions & 6 deletions implementations/rust/ockam/ockam_core/src/cbor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub(crate) mod schema;

use crate::compat::vec::Vec;
use crate::Result;
use minicbor::{CborLen, Encode, Encoder};
use minicbor::{CborLen, Encode};

/// Encode a type implementing [`Encode`] and return the encoded byte vector.
///
Expand All @@ -17,9 +17,10 @@ pub fn cbor_encode_preallocate<T>(x: T) -> Result<Vec<u8>>
where
T: Encode<()> + CborLen<()>,
{
let output_len = minicbor::len(&x);
let output = Vec::with_capacity(output_len);
let mut e = Encoder::new(output);
x.encode(&mut e, &mut ())?;
Ok(e.into_writer())
// Due to minicbor bug this value is bigger than should be for structures having
// #[cbor(transparent)] attribute.
let max_expected_len = minicbor::len(&x);
let mut output = Vec::with_capacity(max_expected_len);
minicbor::encode(x, &mut output)?;
Ok(output)
}
4 changes: 4 additions & 0 deletions implementations/rust/ockam/ockam_transport_core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ pub enum TransportError {
InvalidProtocolVersion,
/// Message is longer than allowed
MessageLengthExceeded,
/// Should not happen
EncodingInternalError,
}

impl ockam_core::compat::error::Error for TransportError {}
Expand All @@ -69,6 +71,7 @@ impl core::fmt::Display for TransportError {
Self::AttackAttempt => write!(f, "excessive length of header, possible DoS attack"),
Self::InvalidProtocolVersion => write!(f, "invalid protocol version"),
Self::MessageLengthExceeded => write!(f, "message length exceeded"),
Self::EncodingInternalError => write!(f, "encoding internal error"),
}
}
}
Expand Down Expand Up @@ -96,6 +99,7 @@ impl From<TransportError> for Error {
AttackAttempt => Kind::Misuse,
InvalidProtocolVersion => Kind::Invalid,
MessageLengthExceeded => Kind::Unsupported,
EncodingInternalError => Kind::Internal,
};

Error::new(Origin::Transport, kind, err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,23 +123,35 @@ impl TcpSendWorker {
// Create a message buffer with prepended length
let transport_message = TcpTransportMessage::from(local_message);

let msg_len = minicbor::len(&transport_message);
// Due to minicbor bug this value is bigger than should be for structures having
// #[cbor(transparent)] attribute. So, this code accounts for that.
let max_expected_payload_len = minicbor::len(&transport_message);

if msg_len > MAX_MESSAGE_SIZE {
return Err(TransportError::MessageLengthExceeded)?;
const LENGTH_VALUE_SIZE: usize = 4; // u32
let mut vec = Vec::with_capacity(LENGTH_VALUE_SIZE + max_expected_payload_len);

// Let's write zeros instead of actual length, since we don't know it yet.
vec.extend_from_slice(&[0u8; LENGTH_VALUE_SIZE]);

// Append encoded payload
minicbor::encode(&transport_message, &mut vec).map_err(|_| TransportError::Encoding)?;

// Should not ever happen...
if vec.len() < LENGTH_VALUE_SIZE {
return Err(TransportError::Encoding)?;
}

// Prepending message with u32 (4 bytes) length
let len = 4 + msg_len;
let payload_len = vec.len() - LENGTH_VALUE_SIZE;

let msg_len_u32 =
u32::try_from(msg_len).map_err(|_| TransportError::MessageLengthExceeded)?;
if payload_len > MAX_MESSAGE_SIZE {
return Err(TransportError::MessageLengthExceeded)?;
}

let mut vec = vec![0u8; len];
let payload_len_u32 =
u32::try_from(payload_len).map_err(|_| TransportError::MessageLengthExceeded)?;

vec[..4].copy_from_slice(&msg_len_u32.to_be_bytes());
minicbor::encode(&transport_message, &mut vec[4..])
.map_err(|_| TransportError::Encoding)?;
// Replace zeros with actual length
vec[..LENGTH_VALUE_SIZE].copy_from_slice(&payload_len_u32.to_be_bytes());

Ok(vec)
}
Expand Down

0 comments on commit 64462e1

Please sign in to comment.