Skip to content

Protobuf Boundary Safety — wire crate#47

Merged
maxholman merged 1 commit intomainfrom
fix/wire-protobuf-boundary-safety
Feb 28, 2026
Merged

Protobuf Boundary Safety — wire crate#47
maxholman merged 1 commit intomainfrom
fix/wire-protobuf-boundary-safety

Conversation

@maxholman
Copy link
Copy Markdown
Contributor

Protobuf Boundary Safety — wire crate

Scope

crates/wire/src/helpers.rs, crates/wire/src/data.rs,
crates/wire/src/socket_set.rs

Out of scope

Behaviour changes beyond error propagation. No changes to callsites outside
crates/wire/.

Why

Two silent data-corruption paths at the protobuf deserialisation boundary:

  1. vec_to_sized_array silently truncates or zero-pads if the input length
    doesn't match N. A 17-byte "IPv4 address" from a malformed protobuf
    message produces a wrong IP address with no error.

  2. port as u16 silently truncates a protobuf u32 port field. A port value
    above 65535 wraps to a wrong port with no error.

Both are suppressed today with #[allow(clippy::cast_possible_truncation)].

What

1. vec_to_sized_array — return Result

Add InvalidLength { expected: usize, got: usize } to ConversionError in
helpers.rs.

Change signature:

pub fn vec_to_sized_array<const N: usize>(vec: &[u8]) -> Result<[u8; N], ConversionError>

Reject any input where vec.len() != N.

The call sites in data.rs are currently infallible From impls — change
them to TryFrom:

  • From<IpV4Address> for std::net::Ipv4AddrTryFrom
  • From<IpV6Address> for std::net::Ipv6AddrTryFrom
  • From<ip_address::IpAddress> for std::net::IpAddrTryFrom

Update any downstream callers that used .into() on these types to use ?.

2. Port truncation — u16::try_from

Add InvalidPort variant to ConversionError.

In socket_set.rs and data.rs, replace every port as u16 with:

u16::try_from(src.port).map_err(|_| ConversionError::InvalidPort)?

Remove:

  • #![allow(clippy::cast_possible_truncation)] at top of socket_set.rs
  • Inline #[allow(clippy::cast_possible_truncation)] at data.rs:167,178

3. Tests

Add #[cfg(test)] tests in helpers.rs:

  • vec_to_sized_array::<4> with a 5-byte input → Err(InvalidLength)
  • vec_to_sized_array::<4> with a 4-byte input → Ok

Add test in socket_set.rs or data.rs:

  • Port value 70_000u32 in a TryFrom impl → Err(InvalidPort)

Notes

  • The task doc references crates/protobuf/ — the crate was renamed to
    crates/wire/. All files are in crates/wire/.
  • just check must pass after the change.

vec_to_sized_array now returns Result and rejects inputs where len != N,
eliminating silent truncation/zero-padding of IP addresses. Port fields
are converted with u16::try_from instead of as-cast, eliminating silent
port wrapping. From impls for IpV4Address/IpV6Address/IpAddr become
TryFrom throughout crates/wire. Removes all cast_possible_truncation
allows. Adds tests for InvalidLength and InvalidPort error paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@maxholman maxholman merged commit 2591896 into main Feb 28, 2026
4 checks passed
@maxholman maxholman deleted the fix/wire-protobuf-boundary-safety branch February 28, 2026 14:11
maxholman added a commit that referenced this pull request May 6, 2026
Closes 8 open dependabot alerts via transitive lockfile bumps:

- rustls-webpki 0.103.9 -> 0.103.13 — CRL/URI/wildcard name-constraint
  handling and panic-on-malformed-CRL DoS (alerts #27 #42 #43 #47)
- rand 0.8.5 -> 0.8.6 and 0.9.2 -> 0.9.4 — soundness fix for callers
  using a custom logger with rand::rng() (#45 #46)
- h3 1.15.8 -> 1.15.11 (website) — path traversal via double-decoded
  %252e%252e in serveStatic and SSE event injection via unsanitized
  carriage return (#24 #25)

No direct dependency edits; all bumps are transitive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant