Skip to content

Add defensive programming in FromBytes::new_box_slice_zeroed #64

Description

@joshlf

In #63, we switched from manual bounds checking plus Layout::from_size_align_unchecked to calling Layout::from_size_align and relying on its bounds checking. There seemed to be a bug in that bounds checking prior to 1.65.0, and so we wrote a test which we disabled on Rust versions prior to 1.65.0. The reasoning was that the worst that could happen was a failed allocation, so it wasn't actually dangerous to expose this bug in the API.

dtolnay/semver#294 deals with this as well, and takes a more defensive stance. It observes that allocation with an invalid Layout is actually UB, and if the API is somehow reachable via attacker-controlled input, it results in an easy-to-exploit path to attacker-controlled UB. I think we may want to add defenses along the same lines.

The specific task is to:

  • Figure out what UB is possible when combining the code as currently written with the version of Layout::from_size_align on 1.64.0
  • Modify FromBytes::new_box_slice_zeroed to ensure that that UB cannot be triggered
  • Leave a // TODO(#67): ... comment to remove the workaround once our MSRV is at least 1.65.0
  • Add that TODO to the list for 1.65.0 kept in List of TODOs blocked on MSRV #67
  • Remove the conditional compilation on test_new_box_slice_zeroed_panics_isize_overflow and update the comment there

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    blocking-next-releaseThis issue should be resolved before we release on crates.ioexperience-hardThis issue is hard, and requires a lot of experience

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions