From 7ac3594044e4a02973cfbbaf8153d0140809db49 Mon Sep 17 00:00:00 2001 From: Dave Grantham Date: Wed, 15 Jul 2026 15:13:56 -0600 Subject: [PATCH] hardening Signed-off-by: Dave Grantham --- .github/workflows/rust.yml | 55 ++++++++++++++++++++++++++++++++----- CHANGELOG.md | 39 +++++++++++++++++++++++++- Cargo.toml | 13 +++++++-- src/base_encoded.rs | 25 ++++++++--------- src/base_encoder.rs | 44 +++++++++++++++--------------- src/base_util.rs | 22 +++++++++++---- src/error.rs | 28 ++++++++++++++++--- src/lib.rs | 27 ++++++++++++------ src/serde/de.rs | 9 +++--- src/serde/mod.rs | 46 +++++++++++++++---------------- src/serde/ser.rs | 2 +- src/varbytes.rs | 56 ++++++++++++++++++++++++++++++-------- src/varuint.rs | 24 ++++++++++++---- 13 files changed, 282 insertions(+), 108 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 9fd45e0..d20f7a0 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -2,21 +2,62 @@ name: Rust on: push: - branches: [ "main" ] + branches: ["main"] pull_request: - branches: [ "main" ] + branches: ["main"] env: CARGO_TERM_COLOR: always jobs: build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Install Rust Toolchain + uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt, clippy + + - name: Check Code Format + run: cargo fmt --all -- --check + - name: Code Lint + run: cargo clippy --all-targets --all-features -- -D warnings + + - name: Build + run: cargo build --verbose --all-features + + - name: Run tests + run: cargo test --verbose --all-features + + msrv: + name: Verify MSRV (1.85) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - name: Build - run: cargo build --verbose - - name: Run tests - run: cargo test --verbose + - uses: actions/checkout@v4 + + - name: Install Rust Toolchain (1.85) + uses: dtolnay/rust-toolchain@1.85.0 + + - name: Check + run: cargo check --all-features + + audit: + name: Security Audit + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Install Rust Toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Install cargo-audit + run: cargo install cargo-audit + + - name: Run cargo audit + run: cargo audit \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1ce6a..f2aef75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,46 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.0.3] - 2026-07-15 + +### Added + +- **`#![deny(unsafe_code)]`** at the crate root. +- **`#[must_use]`** on `Error::custom()`. +- **MSRV declared**: `rust-version = "1.85"` in `Cargo.toml`. CI verifies the + MSRV with a dedicated job. +- **`cargo audit`** job in CI. +- **`cargo fmt --check`** and **`clippy -D warnings`** steps in CI. +- **Clippy lint configuration**: `[lints.clippy]` with `pedantic`, `nursery`, + and `cargo` groups (all `warn`), plus `[lints.rust] unsafe_code = "deny"`. +- **`no_std` documentation** in the crate root noting that the crate is + currently std-only and the `serde` alloc feature config does not make the + crate `no_std`. +- **`Varuint` module documentation** explaining the relationship between + `Varuint` and the integer trait impls in `multi-trait`. +- **`# Errors`** doc section on `BaseEncoder::from_base_encoded`. +- **`#[inline]`** on `Deref`/`DerefMut` impls (replaced `#[inline(always)]`). + +### Changed + +- **Edition 2024**: Updated from Rust 2021. +- **`Varbytes::encode_into`**: Replaced `v.append(&mut self.0.clone())` with + `v.extend_from_slice(&self.0)` to avoid cloning the entire payload. +- **`Varbytes` serde `Serialize`**: Replaced + `v.append(&mut self.as_bytes().to_vec())` with + `v.extend_from_slice(self.as_bytes())` to avoid an intermediate allocation. +- **`DetectedEncoder::from_base_encoded`**: Bails on the first strict decode + success instead of collecting all successful decodings, avoiding O(n) + redundant decodes and false positives from overlapping alphabets. +- **`DetectedEncoder` doc comment**: Fixed typos and formatting. +- **Clippy pedantic/nursery/cargo warnings** resolved across all source. + ## [1.0.0] - 2026-07-13 ### Changed - Synced from bettersign workspace (bs-multiutil 0.7.0) - Renamed crate from `bs-multiutil` to `multi-util` -- Initial published release on crates.io as `multi-util` \ No newline at end of file +- Initial published release on crates.io as `multi-util` + +[1.0.3]: https://github.com/cryptidtech/multi-util/compare/v1.0.0...v1.0.3 +[1.0.0]: https://github.com/cryptidtech/multi-util/releases/tag/v1.0.0 \ No newline at end of file diff --git a/Cargo.toml b/Cargo.toml index 88c983f..ccefa14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,8 @@ [package] name = "multi-util" -version = "1.0.2" -edition = "2021" +version = "1.0.3" +edition = "2024" +rust-version = "1.85" authors = ["Dave Huseby "] description = "Multiformat utility functions and types" repository = "https://github.com/cryptidtech/multi-util.git" @@ -27,3 +28,11 @@ proptest = "1.4" serde_cbor = "0.11" serde_json = "1.0" serde_test = "1.0" + +[lints.clippy] +pedantic = { level = "warn", priority = -1 } +nursery = { level = "warn", priority = -1 } +cargo = { level = "warn", priority = -1 } + +[lints.rust] +unsafe_code = "deny" diff --git a/src/base_encoded.rs b/src/base_encoded.rs index f1edcf8..678a001 100644 --- a/src/base_encoded.rs +++ b/src/base_encoded.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - error::BaseEncodedError, prelude::Base, BaseEncoder, EncodingInfo, Error, MultibaseEncoder, + BaseEncoder, EncodingInfo, Error, MultibaseEncoder, error::BaseEncodedError, prelude::Base, }; use core::{ cmp::Ordering, @@ -12,7 +12,7 @@ use core::{ /// Smart pointer for multibase encoded data. This supports encoding to and /// decoding from multibase encoding strings using [`TryFrom<&str>`] and -/// ['to_string()'] +/// [`to_string()`](core::fmt::Display::to_string) #[derive(Clone)] pub struct BaseEncoded where @@ -29,8 +29,8 @@ where T: EncodingInfo, Enc: BaseEncoder, { - /// Construct a new BaseEncoded instance with the given base - pub fn new(base: Base, t: T) -> Self { + /// Construct a new `BaseEncoded` instance with the given base + pub const fn new(base: Base, t: T) -> Self { Self { base, t, @@ -98,22 +98,22 @@ where } } -impl PartialEq> for BaseEncoded +impl PartialEq for BaseEncoded where T: EncodingInfo + PartialEq + ?Sized, Enc: BaseEncoder, { - fn eq(&self, other: &BaseEncoded) -> bool { + fn eq(&self, other: &Self) -> bool { self.base == other.base && self.t == other.t } } -impl PartialOrd> for BaseEncoded +impl PartialOrd for BaseEncoded where T: EncodingInfo + PartialEq + PartialOrd + ?Sized, Enc: BaseEncoder, { - fn partial_cmp(&self, other: &BaseEncoded) -> Option { + fn partial_cmp(&self, other: &Self) -> Option { self.t.partial_cmp(&other.t) } } @@ -141,8 +141,7 @@ where Enc: BaseEncoder, { fn hash(&self, state: &mut H) { - let s = self.to_string(); - s.hash(state); + self.to_string().hash(state); } } @@ -153,7 +152,7 @@ where { type Target = T; - #[inline(always)] + #[inline] fn deref(&self) -> &Self::Target { &self.t } @@ -164,7 +163,7 @@ where T: EncodingInfo, Enc: BaseEncoder, { - #[inline(always)] + #[inline] fn deref_mut(&mut self) -> &mut Self::Target { &mut self.t } @@ -190,6 +189,6 @@ where Enc: BaseEncoder, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{} - {:?}", Enc::debug_string(self.base), self.t,) + write!(f, "{} - {:?}", Enc::debug_string(self.base), self.t) } } diff --git a/src/base_encoder.rs b/src/base_encoder.rs index c2a5d96..cd016f8 100644 --- a/src/base_encoder.rs +++ b/src/base_encoder.rs @@ -1,9 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - base_name, + BaseIter, Error, base_name, error::{BaseEncodedError, BaseEncoderError}, prelude::Base, - BaseIter, Error, }; /// a trait for base encoding implementations @@ -11,7 +10,12 @@ pub trait BaseEncoder { /// convert a &[u8] to a base encoded value fn to_base_encoded(base: Base, b: &[u8]) -> String; - /// convert a base encoded value to a `Vec` + /// convert a base encoded value to a `Vec`. + /// + /// # Errors + /// + /// Returns an error if the input cannot be decoded as a valid base-encoded + /// value for any of the encoder's supported bases. fn from_base_encoded(s: &str) -> Result)>, Error>; /// get the debug string for the given base @@ -32,7 +36,7 @@ impl BaseEncoder for MultibaseEncoder { fn from_base_encoded(s: &str) -> Result)>, Error> { // try permissive multibase decoding Ok(vec![ - multi_base::decode(s, false).map_err(|_| BaseEncodedError::ValueFailed)? + multi_base::decode(s, false).map_err(|_| BaseEncodedError::ValueFailed)?, ]) } fn debug_string(base: Base) -> String { @@ -43,7 +47,7 @@ impl BaseEncoder for MultibaseEncoder { } } -/// a bare Base58Btc encoder implementation for use with legacy CIDs +/// a bare `Base58Btc` encoder implementation for use with legacy CIDs #[derive(Clone, Debug, Eq, PartialEq)] pub struct Base58Encoder {} @@ -55,7 +59,7 @@ impl BaseEncoder for Base58Encoder { // try strict Base58Btc decoding match Base::Base58Btc.decode(s, true) { Ok(v) => Ok(vec![(Base::Base58Btc, v)]), - Err(e) => Err(BaseEncoderError::Base58(format!("{:?}", e)).into()), + Err(e) => Err(BaseEncoderError::Base58(format!("{e:?}")).into()), } } fn debug_string(_base: Base) -> String { @@ -70,10 +74,11 @@ impl BaseEncoder for Base58Encoder { } } -/// a speculative encoder that tries to detect the correct encoding and decode it -/// encoding is always done using multibase so this does not support symetric -/// decode/encode round trips. this is useful for decoding CIDs that might be -/// base58 encoded "legacy" CIDs but alsy may be multibase encoded CIDs. +/// A speculative encoder that tries to detect the correct encoding and decode it. +/// +/// Encoding is always done using multibase so this does not support symmetric +/// decode/encode round trips. This is useful for decoding CIDs that might be +/// base58 encoded "legacy" CIDs but also may be multibase encoded CIDs. #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct DetectedEncoder {} @@ -82,28 +87,23 @@ impl BaseEncoder for DetectedEncoder { multi_base::encode(base, b) } fn from_base_encoded(s: &str) -> Result)>, Error> { - // first try permissive multibase decoding + // First try permissive multibase decoding (prefix-based). if let Ok((base, data)) = multi_base::decode(s, false) { return Ok(vec![(base, data)]); } - // start at the Identity base so we skip it + // Start after the Identity base so we skip it. let iter: BaseIter = Base::Identity.into(); - // next try "naked" encoding in increasing symbol space size order. - // these have to be strict decodings to avoid confusion - let mut v = Vec::default(); + // Try "naked" encoding in increasing symbol-space size order. + // Use strict decoding and bail on the first success to avoid + // false positives from bases with overlapping alphabets. for encoding in iter { if let Ok(data) = encoding.decode(s, true) { - v.push((encoding, data)); + return Ok(vec![(encoding, data)]); } } - if v.is_empty() { - // raise an error - Err(BaseEncodedError::ValueFailed.into()) - } else { - Ok(v) - } + Err(BaseEncodedError::ValueFailed.into()) } fn debug_string(base: Base) -> String { format!("{} ('{}')", base_name(base), base.code()) diff --git a/src/base_util.rs b/src/base_util.rs index e4cf093..5963560 100644 --- a/src/base_util.rs +++ b/src/base_util.rs @@ -2,8 +2,14 @@ use crate::prelude::Base; /// convert a multibase Base to its string equivalent +#[must_use] pub fn base_name(b: Base) -> String { - use Base::*; + use Base::{ + Base2, Base8, Base10, Base16Lower, Base16Upper, Base32HexLower, Base32HexPadLower, + Base32HexPadUpper, Base32HexUpper, Base32Lower, Base32PadLower, Base32PadUpper, + Base32Upper, Base32Z, Base36Lower, Base36Upper, Base58Btc, Base58Flickr, Base64, Base64Pad, + Base64Url, Base64UrlPad, Base256Emoji, Identity, + }; match b { Identity => "Identity", Base2 => "Base2", @@ -43,8 +49,9 @@ impl Default for BaseIter { } impl BaseIter { - /// create a new BaseIter - pub fn new() -> Self { + /// create a new `BaseIter` + #[must_use] + pub const fn new() -> Self { Self(None) } } @@ -59,7 +66,12 @@ impl Iterator for BaseIter { type Item = Base; fn next(&mut self) -> Option { - use Base::*; + use Base::{ + Base2, Base8, Base10, Base16Lower, Base16Upper, Base32HexLower, Base32HexPadLower, + Base32HexPadUpper, Base32HexUpper, Base32Lower, Base32PadLower, Base32PadUpper, + Base32Upper, Base32Z, Base36Lower, Base36Upper, Base58Btc, Base58Flickr, Base64, + Base64Pad, Base64Url, Base64UrlPad, Base256Emoji, Identity, + }; let result = match self.0 { None => Identity, Some(b) => match b { @@ -106,7 +118,7 @@ mod tests { #[test] fn test_last_iter() { let mut iter: BaseIter = Base::Base256Emoji.into(); - assert_eq!(iter.next(), None) + assert_eq!(iter.next(), None); } #[test] diff --git a/src/error.rs b/src/error.rs index 892513e..31506b5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,10 +9,10 @@ pub enum Error { /// Multicodec decode error #[error(transparent)] Multicodec(#[from] multi_codec::Error), - /// BaseEncoded error + /// `BaseEncoded` error #[error(transparent)] BaseEncoded(#[from] BaseEncodedError), - /// BaseEncoder error + /// `BaseEncoder` error #[error(transparent)] BaseEncoder(#[from] BaseEncoderError), /// Custom error for inner types to use when nothing else works @@ -43,12 +43,32 @@ pub enum Error { /// The actual number of bytes available in the buffer actual: usize, }, + /// Decoded length exceeds the configured maximum + /// + /// This error occurs when a varint length claim exceeds [`crate::varbytes::MAX_DECODED_SIZE`]. + /// Enforcing an upper bound on a single decoded `Varbytes` value prevents a peer that supplies + /// a legitimately-sized buffer from causing the decoder to allocate an arbitrarily large + /// `Vec` (`DoS` via memory exhaustion). The default ceiling is 16 MiB, which comfortably + /// exceeds every legitimate multiformat payload in this stack while still bounding the + /// worst-case allocation for untrusted wire data. + /// + /// # Security + /// + /// This validation mitigates CWE-400 (Uncontrolled Resource Consumption). + #[error("decoded length {claimed} exceeds maximum {max}")] + InputTooLarge { + /// The number of bytes claimed by the length prefix + claimed: usize, + /// The configured maximum decoded size + max: usize, + }, } impl Error { /// create a custom error instance + #[must_use] pub fn custom(s: &str) -> Self { - Error::Custom(s.to_string()) + Self::Custom(s.to_string()) } } @@ -56,7 +76,7 @@ impl Error { #[derive(Clone, Debug, thiserror::Error)] #[non_exhaustive] pub enum BaseEncodedError { - /// BaseEncoder error + /// `BaseEncoder` error #[error(transparent)] BaseEncoder(#[from] BaseEncoderError), /// Value decoding failed diff --git a/src/lib.rs b/src/lib.rs index cd4e2ea..9486fd0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,30 +1,41 @@ // SPDX-License-Identifier: Apache-2.0 -//! multiutil +//! # multi-util +//! +//! Multiformat utility functions and types. +//! +//! ## `no_std` Note +//! +//! This crate is currently std-only. The `serde` dependency is configured with +//! `default-features = false, features = ["alloc"]` so that the serde layer +//! itself does not pull in std, but the crate as a whole still requires std +//! via `thiserror` and the lack of a `#![no_std]` crate attribute. A future +//! release may introduce a `std` feature gate and full `no_std` support. #![warn(missing_docs)] #![deny( + unsafe_code, trivial_casts, trivial_numeric_casts, unused_import_braces, unused_qualifications )] -/// BaseEncoded smart pointer +/// `BaseEncoded` smart pointer pub mod base_encoded; pub use base_encoded::BaseEncoded; -/// BaseEncoder trait and impls +/// `BaseEncoder` trait and impls pub mod base_encoder; pub use base_encoder::{Base58Encoder, BaseEncoder, DetectedEncoder, MultibaseEncoder}; /// Base related utility functions / types pub mod base_util; -pub use base_util::{base_name, BaseIter}; +pub use base_util::{BaseIter, base_name}; -/// CodecInfo trait +/// `CodecInfo` trait pub mod codec_info; pub use codec_info::CodecInfo; -/// EncodingInfo trait +/// `EncodingInfo` trait pub mod encoding_info; pub use encoding_info::EncodingInfo; @@ -119,8 +130,8 @@ mod test { } impl From for Vec { - fn from(unit: Unit) -> Vec { - let mut v = Vec::default(); + fn from(unit: Unit) -> Self { + let mut v = Self::default(); v.extend_from_slice(&unit.0); v } diff --git a/src/serde/de.rs b/src/serde/de.rs index e58d184..9c3b93a 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -69,14 +69,13 @@ where let base = match seq.next_element::()? { Some(b) => Base::from_code(b).map_err(|e| de::Error::custom(e.to_string()))?, None => { - return Err(de::Error::custom("expected base encoding char".to_string())) + return Err(de::Error::custom("expected base encoding char".to_string())); } }; - let t = match seq.next_element()? { - Some(t) => t, - None => return Err(de::Error::custom("expected inner type value".to_string())), - }; + let t = seq + .next_element()? + .ok_or_else(|| de::Error::custom("expected inner type value".to_string()))?; Ok(Self::Value { enc: marker::PhantomData, diff --git a/src/serde/mod.rs b/src/serde/mod.rs index fbf5c2d..615be35 100644 --- a/src/serde/mod.rs +++ b/src/serde/mod.rs @@ -7,7 +7,7 @@ mod ser; mod tests { use crate::prelude::*; use serde::{Deserialize, Serialize}; - use serde_test::{assert_tokens, Configure, Token}; + use serde_test::{Configure, Token, assert_tokens}; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] struct Unit((u8, [u8; 2])); @@ -16,7 +16,7 @@ mod tests { impl Unit { fn encoded_default() -> EncodedUnit { - EncodedUnit::new(Unit::preferred_encoding(), Unit::default()) + EncodedUnit::new(Self::preferred_encoding(), Self::default()) } } @@ -49,10 +49,10 @@ mod tests { } impl From for Vec { - fn from(unit: Unit) -> Vec { - let mut v: Vec = Vec::default(); - v.push(unit.0 .0); - v.extend_from_slice(&unit.0 .1); + fn from(unit: Unit) -> Self { + let mut v: Self = Self::default(); + v.push(unit.0.0); + v.extend_from_slice(&unit.0.1); v } } @@ -137,49 +137,49 @@ mod tests { #[test] fn test_u8_varuint() { let v = Varuint(0x01_u8); - assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]); } #[test] fn test_u8_long_varuint() { let v = Varuint(0xFF_u8); - assert_tokens(&v, &[Token::BorrowedBytes(&[0xFF, 0x01])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0xFF, 0x01])]); } #[test] fn test_u16_varuint() { let v = Varuint(0x0100_u16); - assert_tokens(&v, &[Token::BorrowedBytes(&[0x80, 0x02])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0x80, 0x02])]); } #[test] fn test_u16_short_varuint() { let v = Varuint(0x0001_u16); - assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]); } #[test] fn test_u16_long_varuint() { let v = Varuint(0xFFFF_u16); - assert_tokens(&v, &[Token::BorrowedBytes(&[0xFF, 0xFF, 0x03])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0xFF, 0xFF, 0x03])]); } #[test] fn test_u32_varuint() { let v = Varuint(0x0100_0000_u32); - assert_tokens(&v, &[Token::BorrowedBytes(&[0x80, 0x80, 0x80, 0x08])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0x80, 0x80, 0x80, 0x08])]); } #[test] fn test_u32_short_varuint() { let v = Varuint(0x0000_0001_u32); - assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]); } #[test] fn test_u32_long_varuint() { let v = Varuint(0xFFFF_FFFF_u32); - assert_tokens(&v, &[Token::BorrowedBytes(&[0xFF, 0xFF, 0xFF, 0xFF, 0x0F])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0xFF, 0xFF, 0xFF, 0xFF, 0x0F])]); } #[test] @@ -190,13 +190,13 @@ mod tests { &[Token::BorrowedBytes(&[ 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01, ])], - ) + ); } #[test] fn test_u64_short_varuint() { let v = Varuint(0x0000_0000_0000_0001_u64); - assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]) + assert_tokens(&v, &[Token::BorrowedBytes(&[0x01])]); } #[test] @@ -207,7 +207,7 @@ mod tests { &[Token::BorrowedBytes(&[ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01, ])], - ) + ); } #[test] @@ -219,13 +219,13 @@ mod tests { 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x02, ])], - ) + ); } #[test] fn test_u128_short_varuint() { let v = Varuint(0x0000_0000_0000_0000_0000_0000_0000_0001_u128); - assert_tokens(&v, &[Token::Bytes(&[0x01])]) + assert_tokens(&v, &[Token::Bytes(&[0x01])]); } #[test] @@ -237,7 +237,7 @@ mod tests { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x03, ])], - ) + ); } #[test] @@ -248,13 +248,13 @@ mod tests { &[Token::Bytes(&[ 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01, ])], - ) + ); } #[test] fn test_usize_short_varuint() { let v = Varuint(0x0000_0000_0000_0001_usize); - assert_tokens(&v, &[Token::Bytes(&[0x01])]) + assert_tokens(&v, &[Token::Bytes(&[0x01])]); } #[test] @@ -265,7 +265,7 @@ mod tests { &[Token::Bytes(&[ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01, ])], - ) + ); } #[test] diff --git a/src/serde/ser.rs b/src/serde/ser.rs index f49602a..df18828 100644 --- a/src/serde/ser.rs +++ b/src/serde/ser.rs @@ -41,7 +41,7 @@ impl ser::Serialize for Varbytes { S: ser::Serializer, { let mut v = self.as_bytes().len().encode_into(); - v.append(&mut self.as_bytes().to_vec()); + v.extend_from_slice(self.as_bytes()); serializer.serialize_bytes(v.as_slice()) } } diff --git a/src/varbytes.rs b/src/varbytes.rs index d61204c..af97bcd 100644 --- a/src/varbytes.rs +++ b/src/varbytes.rs @@ -5,34 +5,49 @@ use multi_base::Base; use multi_trait::prelude::{EncodeInto, TryDecodeFrom}; /// A wrapper type to handle serde of byte arrays as bytes -#[derive(Clone, Default, PartialEq)] +#[derive(Clone, Default, PartialEq, Eq)] pub struct Varbytes(Vec); +/// Maximum number of bytes a single [`Varbytes`] value will allocate when +/// decoded from untrusted wire data. +/// +/// The 16 MiB ceiling comfortably exceeds every legitimate multiformat payload +/// handled by this crate stack (the largest is a Classic `McEliece` secret key +/// at a few hundred KiB) while bounding the worst-case allocation an attacker +/// can trigger with a crafted length prefix. Callers that need a different +/// bound should validate the raw buffer length before invoking +/// [`Varbytes::try_decode_from`]. +pub const MAX_DECODED_SIZE: usize = 16 * 1024 * 1024; + /// type alias for a Varbytes base encoded to/from string pub type EncodedVarbytes = BaseEncoded; impl Varbytes { /// Create a new Varbytes from a Vec - pub fn new(data: Vec) -> Self { + #[must_use] + pub const fn new(data: Vec) -> Self { Self(data) } /// create an encoded varbytes - pub fn encoded_new(base: Base, v: Vec) -> EncodedVarbytes { - BaseEncoded::new(base, Varbytes::new(v)) + #[must_use] + pub const fn encoded_new(base: Base, v: Vec) -> EncodedVarbytes { + BaseEncoded::new(base, Self::new(v)) } /// Get a reference to the inner byte slice + #[must_use] pub fn as_bytes(&self) -> &[u8] { &self.0 } /// Get a mutable reference to the inner byte vector - pub fn as_bytes_mut(&mut self) -> &mut Vec { + pub const fn as_bytes_mut(&mut self) -> &mut Vec { &mut self.0 } /// consume self and return inner vec + #[must_use] pub fn to_inner(self) -> Vec { self.0 } @@ -47,7 +62,7 @@ impl fmt::Debug for Varbytes { impl ops::Deref for Varbytes { type Target = Vec; - #[inline(always)] + #[inline] fn deref(&self) -> &Self::Target { &self.0 } @@ -64,7 +79,7 @@ impl EncodingInfo for Varbytes { } impl From for Vec { - fn from(vb: Varbytes) -> Vec { + fn from(vb: Varbytes) -> Self { vb.encode_into() } } @@ -72,7 +87,7 @@ impl From for Vec { impl EncodeInto for Varbytes { fn encode_into(&self) -> Vec { let mut v = self.0.len().encode_into(); - v.append(&mut self.0.clone()); + v.extend_from_slice(&self.0); v } } @@ -92,6 +107,16 @@ impl<'a> TryDecodeFrom<'a> for Varbytes { fn try_decode_from(bytes: &'a [u8]) -> Result<(Self, &'a [u8]), Self::Error> { let (len, ptr) = usize::try_decode_from(bytes)?; + // Reject length claims that exceed the configured maximum decoded size. + // This bounds the worst-case allocation for untrusted wire data and + // mitigates CWE-400 (Uncontrolled Resource Consumption). + if len > MAX_DECODED_SIZE { + return Err(Error::InputTooLarge { + claimed: len, + max: MAX_DECODED_SIZE, + }); + } + // Validate buffer has enough data for claimed length // This prevents buffer overflow (CWE-125) when length claim exceeds available data if len > ptr.len() { @@ -149,7 +174,7 @@ mod test { #[test] fn test_debug() { let v = Varbytes::new(vec![1, 2, 3]); - assert_eq!("[3, 1, 2, 3]".to_string(), format!("{:?}", v)); + assert_eq!("[3, 1, 2, 3]".to_string(), format!("{v:?}")); } // ============================================================================ @@ -183,13 +208,20 @@ mod test { "Should reject length claim that exceeds available data" ); - // Verify correct error type + // Verify correct error type. The 4GB claim exceeds both the + // MAX_DECODED_SIZE cap (16 MiB) and the available buffer (3 bytes); + // either InputTooLarge or InsufficientData is an acceptable rejection + // — both prevent the out-of-bounds read. match result.unwrap_err() { + Error::InputTooLarge { claimed, max } => { + assert_eq!(claimed, huge_length); + assert_eq!(max, MAX_DECODED_SIZE); + } Error::InsufficientData { expected, actual } => { assert_eq!(expected, huge_length); assert_eq!(actual, 3); } - e => panic!("Expected InsufficientData error, got: {:?}", e), + e => panic!("Expected InputTooLarge or InsufficientData error, got: {e:?}"), } } @@ -371,7 +403,7 @@ mod test { prop_assert_eq!(expected, claimed_len); prop_assert_eq!(actual, actual_len); } - e => return Err(TestCaseError::fail(format!("Wrong error type: {:?}", e))), + e => return Err(TestCaseError::fail(format!("Wrong error type: {e:?}"))), } } } diff --git a/src/varuint.rs b/src/varuint.rs index 964b17a..28a7318 100644 --- a/src/varuint.rs +++ b/src/varuint.rs @@ -1,11 +1,25 @@ // SPDX-License-Identifier: Apache-2.0 +//! # Varuint +//! +//! A wrapper type for handling serde serialization of numeric types as varuint +//! bytes. +//! +//! ## Relationship to `multi-trait` +//! +//! [`Varuint`] is a thin newtype that delegates encoding/decoding to the +//! `EncodeInto` and `TryDecodeFrom` implementations that `multi-trait` +//! provides for the integer types themselves. It exists primarily to give +//! numeric values a concrete `EncodingInfo` impl and a serde-friendly +//! representation, not to define a separate varint encoding. In other words, +//! `Varuint` and the integer trait impls in `multi-trait` are two abstraction +//! levels over the same underlying `unsigned-varint` codec. use crate::{BaseEncoded, EncodingInfo, Error}; use core::{fmt, ops}; use multi_base::Base; use multi_trait::{EncodeInto, TryDecodeFrom}; /// A wrapper type to handle serde of numeric types as varuint bytes -#[derive(Clone, PartialEq)] +#[derive(Clone, PartialEq, Eq)] pub struct Varuint(pub T); /// type alias for a Varuint base encoded to/from string @@ -16,7 +30,7 @@ where T: EncodeInto + for<'a> TryDecodeFrom<'a>, { /// create a new encoded varuint - pub fn encoded_new(base: Base, t: T) -> EncodedVaruint { + pub const fn encoded_new(base: Base, t: T) -> EncodedVaruint { BaseEncoded::new(base, Self(t)) } @@ -47,7 +61,7 @@ where impl ops::Deref for Varuint { type Target = T; - #[inline(always)] + #[inline] fn deref(&self) -> &T { &self.0 } @@ -67,7 +81,7 @@ impl From> for Vec where T: EncodeInto, { - fn from(vu: Varuint) -> Vec { + fn from(vu: Varuint) -> Self { vu.0.encode_into() } } @@ -139,6 +153,6 @@ mod test { #[test] fn test_debug() { let v = Varuint(0xed_u64); - assert_eq!("[237, 1]".to_string(), format!("{:?}", v)); + assert_eq!("[237, 1]".to_string(), format!("{v:?}")); } }