Pre-RFC: Trait for deserializing untrusted input

libs
1
  • Feature Name: arbitrary_bytes_safe_trait
  • Start Date: (fill me in with today’s date, YYYY-MM-DD)
  • RFC PR: (leave this empty)
  • Rust Issue: (leave this empty)

Summary

Introduce an unsafe marker trait that guarantees that any byte sequence of size_of::<T>() bytes is a valid instance of T, and a derive for that trait.

Motivation

In deserializing input from untrusted sources (IPC, disk, network, etc), it is sometimes desirable to be able to interpret a sequence of bytes as a particular type. In general, this is unsafe, but for a large subset of types, it is safe. Currently, code that wishes to do this needs to be unsafe. It would be a big ergonomics win if the users and authors of deserialization APIs could deserialize such types without needing to reason about memory safety themselves.

Guide-level Explanation

Introduce a marker trait, pub unsafe trait ArbitraryBytesSafe {} (name to be bikeshedded) that indicates that any sequence of size_of::<T>() bytes is a valid instance of T.

Provide four (safe!) associated functions with default impsl on ArbitraryBytesSafe:

  • fn transmute_ref<T>(t: &T) -> &Self
  • fn transmute_mut<T: ArbitraryBytesSafe>(t: &mut T) -> &mut Self
  • fn transmute_slice_ref<T>(slc: &[T]) -> &Self
  • fn transmute_slice_mut<T: ArbitraryBytesSafe>(slc: &mut [T]) -> &mut Self

The first two functions ensure at compile time that size_of::<T>() == size_of::<Self>(). The second two functions check at runtime that slc.len() * size_of::<T>() == size_of::<Self>().

Provide a custom derive for ArbitraryBytesSafe.

Reference-level Explanation

TODO: Explain how transmute_XXX is implemented.

Note that, when obtaining an immutable reference, T (the source type) doesn’t need to be ArbitraryBytesSafe because it isn’t mutated. When obtaining a mutable reference, it does need to be ArbitraryBytesSafe because there’s no guarantee that valid instances of Self correspond to the bits of valid instances of T without this trait bound.

The custom derive uses the following rules to determine whether a type is ArbitraryBytesSafe:

  • The primitives uXXX and iXXX are safe, including usize and isize.
  • Arrays are safe if their element types are safe.
  • Structs and tuple structs are safe if all of their fields are safe and if it is guaranteed that there is no internal padding. This is true if repr(packed) is used or if repr(C) is used and the alignment of all fields is met without needing any padding (e.g., if a u32 follows two u16s). For tuple structs, this is also true if repr(transparent) is used. Anonymous tuples are never safe because there is no way to specify a repr on them.
  • Enums are safe if they are C-like, have either repr(C) or repr(i*)/repr(u*), and if every possible discriminant value corresponds to a variant.

Rationale and Alternatives

This API allows for clean, safe serialization APIs (that don’t require unsafe impls internally) such as:

  • For reading from a stream, fn read<T: ArbitraryBytesSafe>(&mut self) -> T
  • For zero-copy parsing of input, fn parse<'a>(input: &'a [u8]) -> Result<Parsed<'a>, ParseErr>

It also has uses beyond deserializing untrusted input such as:

  • Generating random instances of things for testing
  • Debugging/inspecting arbitrary regions of program memory

Alternatives:

  • Do not have ArbitraryBytesSafe::transmute_XXX, and simply allow unsafe code to use ArbitraryBytesSafe as a marker trait that provides it the guarantees it needs to do unsafe things such as create an uninitialized T and then copy bytes into it manually. Downside: you lose the guarantee that the sizes match.
  • Instead of a custom derive, use a normal macro that is placed around the type definition and can decide whether or not to emit unsafe impl ArbitraryBytesSafe for <type> {}. This is less ergonomic, but also easier to implement as a first pass.

Prior Art

Thanks to @dtolnay for pointing out the Pod trait, which is similar to ArbitraryBytesSafe, although it performs more checks at runtime (in particular, size and alignment checking). It also provides a large number of utility functions available to Pod types.

The recent FromBits/IntoBits pre-RFC dicusses a different but related case - conversions between types in which, while arbitrary bit patterns may not be safe, all of the valid bit patterns of one type correspond to valid bit patterns of the other type, and so the conversion is nonetheless safe.

Unresolved Questions

  • How do the transmute_XXX functions guarantee that the alignment of their inputs are at least as large as the alignment requirements of Self? Some options might be:
    • At compile time, check not only that the sizes match, but also that the alignment of T is at least as large as the alignment of Self
    • Have transmute_XXX check at runtime. The user will be encouraged to check themselves, in which case the second check (inside transmute_XXX) will be redundant and optimized out.
    • Introduce transmute_XXX_aligned which guarantees that align_of::<T>() >= align_of::<Self>() at compile time, while transmute_XXX checks at runtime
    • The same as the previous bullet, except that transmute_XXX are unsafe, and the documentation states that the only precondition is that alignments are guaranteed by the caller
    • Require that only types with an alignment of 1 can be ArbitraryBytesSafe.
    • Provide a transmute<T>(t: &T) -> Self that returns Self by value, and so has no alignment requirements
  • Should the requirement be that the size of the input is the same size as Self, or just that it’s no smaller than Self? In other words, is it valid to transmute a reference to an object of n bytes to a reference to an object of m < n bytes?
  • Should we add a restriction that ArbitraryBytesSafe types have trivial drops? My intuition is that this is not needed.
  • Should transmute_XXX be associated functions of ArbitraryBytesSafe or instead a library functions (or something else entirely?)
  • Does transmute imply unsafety (since mem::transmute is the canonical unsafe function)? Is there a better word to use?
6 Likes
2

This proposal seems like it could be implemented entirely in a library. Did I miss something that requires it to be in std? Or some reason why experimenting with this in a crate wouldn’t be enough to prove whether it’s worth adding to std?

5 Likes
3

I'd love to implement it in a library if possible. However, I'm not sure how you'd ensure that transmute_XXX can only be called if certain requirements are imposed on their argument types. Put another way, I believe that mem::transmute's requirement that the input and output sizes match requires explicit compiler support, and I believe the transmute_XXX proposed here would require compiler support for the same reason.

EDIT: Another option would be to make transmute_XXX unsafe and have users go through a macro which first static_assert!s the guarantees we need before calling transmute_XXX. That is implementable in a library w/o compiler help.

4

I like this idea. I’ve seen variations on this theme as well, in the form of traits for types whose range includes all possible values of their underlying bytes; there are proposals to make unions consisting entirely of such fields safe in some circumstances.

As a nit, I wouldn’t describe it as “untrusted” input so much as “arbitrary” input.

For structures and tuples, this property should hold not if repr(C) and all fields have their “natural” alignment, not just if they have alignment of 1. For instance, the following struct should naturally have this trait:

#[repr(C)]
struct Foo {
    x: u16,
    y: u16,
    z: u32,
}
5

I originally had that, although I think that arbitrary is more descriptive since you could technically do something like use this to generate random instances of things. I'll add an example like that in the proposal.

Ah, good point. I'll update the proposal.

6

Prior art: the Plain Old Data trait which I have found frequently useful. It provides tons of safe abstractions like Pod::map_slice which converts any &[T] to &[U] where T and U are plain old data.

2 Likes
7

The Pod trait looks quite promising, yes!

8

My understanding is that POD usually just refers to Copy types since you can copy them w/o any special logic - they're just plain old bits (data). E.g., this explanation. It seems like the use of the term in that crate ascribes extra meaning to it? Not that that's a bad thing - that crate looks really cool! Just making sure I understand the terminology properly.

Also, that crate makes some different decisions that are worth calling out (particularly in case you think they did it the right way):

  • They only allow you to convert between Pod types, rather than converting any type (Pod or not) into a Pod type.
  • All of their methods check size and alignment at runtime, returning Options. This crate's code appears from 3 years ago, when static_assert! didn't exist, so I can't blame them for that.
  • It's not clear to me what Pod means here. I think it means the same as what I'm proposing ArbitraryBytesSafe to mean, although I'm not positive, especially since Pod::uninitialized is unsafe, which it wouldn't need to be under the definition proposed here.
9

Is there a reason you want to force all structs to have to impl it instead of being an auto trait like Send and Sync?

10

Because it’s not possible to automatically infer whether an arbitrary sequence of bytes is indeed a valid value for a type. That is a guarantee provided by the invariants and methods of the type, and it has to be explicitly designed for.

3 Likes
11

Won't const generics (+ equality constraints) provide a general solution to this problem?

I would rather extend the type system to make this kind of constraint possible, instead of adding more special cases to the language.

The magic behind transmute was a reasonable compromise for Rust 1.0, given the state of constant expressions at the time. But I don't think its approach is something we should emulate going forward, especially when a cleaner approach is on the horizon.

2 Likes
12

This reminds me of Safe conversions for DSTs, since in some ways it’s just ReinterpretableFrom<[u8; sizeof(Self)]>.

13

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.