`as` cast in a real vulnerability

language design
21

Yes, it makes sense to have this. It would panic on overflow in debug mode and wrap in release mode.

Proposed name: narrowing_cast.

4 Likes
22

Rust usually nudges the user towards the correct solution via ergonomics, but u16::MAX as usize is so much cleaner than usize::from(u16::MAX) — especially in the case of something like u8::try_from(my_u16).unwrap() compared to my_u16 as u8 — that users can be forgiven for using the wrong one.

Personally I would like it if (at least in my own code) all casts to numeric types were forbidden and I were required to go through From/TryFrom. As far as I'm concerned, as should only be legal for 100% data-preserving casts such as to function pointers and dyn Trait.

4 Likes
23

Something that could help reduce the usage of the as cast would be to implement [u/i]8/16/32/64/… to [u/i]size behind some kind of #[bikesheed_isize_is_more_than_xxx_bits], and likewise for the reverse operation.

This way my_i32 as isize could be replaced by isize::from(my_i32) instead of the way too verbose isize::try_from(my_i32).unwrap(). If so, I think as cast could be a warn-by-default in many (if not all) cases.

4 Likes
24

Casting u32 to usize is a hack that's only required because there is no support for 32-bit-indexed vectors and slices. The problem could instead be solved by a third-party crate that contains Array32<T, LEN>, Vec32<T> and Slice32<T> all indexed by u32.

I was going to create something like this but I don't see how to even define Array32 nicely because neither of these compiles due to limitations of const:

struct Array32<T, const LEN: u32>([T; usize::try_from(LEN).unwrap()]);
struct Array32<T, const LEN: u32>([T; LEN as usize]);
25

Maybe we could make casting function pointers, raw pointers and others unsafe? So this would produce a compile time error.

26

I think that being able to write:

u16::MAX.into::<usize>()
  • on u16::max.into::<usize>() misuse:
    error[E0277]: the trait bound `usize: From<fn(u16, u16) -> u16 {<u16 as std::cmp::Ord>::max}>` is not satisfied
  --> src/lib.rs:24:14
   |
24 |     u16::max.into::<usize>()
   |              ^^^^ the trait `From<fn(u16, u16) -> u16 {<u16 as std::cmp::Ord>::max}>` is not implemented for `usize`
   |

would be the ideal scenario.

In a completely imaginary alternative syntax scenario (or parallel Rust universe, if you want), we could envision/have envisioned having the Into trait defined as:

trait Into<Dst> {
    //      make outer generic turbofishable
    //      vvvvvvvvv
    fn into<super Dst>(self) -> Dst;
}
  • explicit impls would be allowed to skip the extra super-"generic" specifier, for back-compat purposes,
  • call-sites would now be able to write u16::MAX.into::<usize>(), whilst also remaining able not to use the turbofish (see fn demo below).

For reference, the latter point can already be achieved in current Rust, but since it breaks the former bullet, it wouldn't be retro-compatible (plus, it is quite verbose and thus ugly):

/// Helper trait for equality constraints.
trait Is { type ItSelf: ?Sized; }
impl<T: ?Sized> Is for T { type ItSelf = T; }

trait Into<Dst>  {
    fn into<TurbofishedDst>(self) -> Dst
    where
        Dst: Is<ItSelf = TurbofishedDst>, // make `TurbofishedDst` inferrable off `Dst` (and thus skippable)
        TurbofishedDst: Is<ItSelf = Dst>, // make `Dst` deducible off `TurbofishedDst`.
    ;
}

fn demo(len: usize) -> bool {
    // works
    let _: usize = u16::MAX.into();
    
    // fails :)
    // u16::max.into::<usize>();
    
    // works too!
    len <= u16::MAX.into::<usize>()
}

I guess a less elegant, but easy-to-retrofit approach, could be:

trait Into<Dst> {
    fn into… // current def

    #[sealed]
    fn to<TurbofishedDst>(self) -> Dst
    where
        TurbofishedDst: Is<ItSelf = Dst>, // make `Dst` deducible off `TurbofishedDst`.
    {
        self.into()
    }
}

But I would not be personally super fond of suddenly having to switch to a less accurate word (.to::<…>() rather than .into::<…>()) just to make the whole thing easier to retrofit (but for those interested, to_trait - Rust offers this very API).

5 Likes
27

Theoretically, a new edition could swap the existing Into trait in the prelude for one with a generic fn into instead. I believe that ever since we re-rebalanced coherence, the reason to impl Into instead of impl From has all but disappeared, and ? even uses From to convert errors, not Into.

Given a time machine, I don't think we would have generic Into<U>; the only reasons for it to be generic at the trait level are that it used to be impossible to implement From<T> for a foreign type, and the syntactic nicety of an impl Into<U> parameter. The former isn't the case anymore, and the latter is a type inference harming anti pattern a majority of the time[1].

  1. This isn't to say it doesn't have its applications. impl AsRef<Path> parameter overloading is almost a necessity for fs function ergonomics. But asking the caller to write .into() sometimes isn't that big of a burden, and allows the concrete parameter type to influence type inference and errors in the caller when they aren't writing .into(). And "did you want to write .into()" is an existing lint. ↩︎

3 Likes
28

I wish that existed as a clippy lint, but the only option we have here currently is deny(clippy::as_conversions) which disallows all as casts.

There's a ton of clippy lints for particular ways of losing data by casting around integers, but even enabling all of them does not catch all int-to-int casts. I suggested we could have such a lint in clippy:

1 Like
29

A possible alternative would be extending the : T syntax used to specify the type of bindings to allow it to be used on expressions too (or some other syntax, this is just the nicest syntax I could think of in a minute), such that expr: T says "expr should have type T" in the exact same way that { let temp: T = expr; temp } does. So instead of spelling it as u16::MAX.into::<usize>(), you would spell it u16::MAX.into(): usize and it would work everywhere.

This more or less solves the problem in the general case "externally" and seems broadly useful.

30

That was called generalized type-ascription and was previously implemented on nightly. But IIRC there are syntactical issues in some positions and even having it unstably supported caused issues for diagnostics, so it got removed.

1 Like
31

I do agree that would be the obvious syntax, but see De-RFC: Remove type ascription by Manishearth · Pull Request #3307 · rust-lang/rfcs · GitHub