Rust soundness fixes (#7518)

* Rust soundness fixes

* Second pass

* Make init_from_table unsafe

* Remove SafeSliceAccess

* Clippy

* Remove create_vector_of_strings

* More clippy

* Remove deprecated root type accessors

* More soundness fixes

* Fix EndianScalar for bool

* Add TriviallyTransmutable

* Add debug assertions

* Review comments

* Review feedback
This commit is contained in:
Raphael Taylor-Davies
2022-09-29 14:58:49 +01:00
committed by GitHub
parent dadbff5714
commit 374f8fb5fb
102 changed files with 2673 additions and 2035 deletions

View File

@@ -1,14 +1,14 @@
use crate::follow::Follow;
use crate::{ForwardsUOffset, SOffsetT, SkipSizePrefix, UOffsetT, VOffsetT, Vector, SIZE_UOFFSET};
#[cfg(feature = "no_std")]
use alloc::vec::Vec;
use core::ops::Range;
use core::option::Option;
use crate::follow::Follow;
use crate::{ForwardsUOffset, SOffsetT, SkipSizePrefix, UOffsetT, VOffsetT, Vector, SIZE_UOFFSET};
#[cfg(feature="no_std")]
use thiserror_core2::Error;
#[cfg(not(feature="no_std"))]
#[cfg(not(feature = "no_std"))]
use thiserror::Error;
#[cfg(feature = "no_std")]
use thiserror_core2::Error;
/// Traces the location of data errors. Not populated for Dos detecting errors.
/// Useful for MissingRequiredField and Utf8Error in particular, though
@@ -28,8 +28,10 @@ pub enum ErrorTraceDetail {
position: usize,
},
}
#[derive(PartialEq, Eq, Default, Debug, Clone)]
pub struct ErrorTrace(Vec<ErrorTraceDetail>);
impl core::convert::AsRef<[ErrorTraceDetail]> for ErrorTrace {
#[inline]
fn as_ref(&self) -> &[ErrorTraceDetail] {
@@ -63,7 +65,7 @@ pub enum InvalidFlatbuffer {
error_trace: ErrorTrace,
},
#[error("String in range [{}, {}) is missing its null terminator.\n{error_trace}",
range.start, range.end)]
range.start, range.end)]
MissingNullTerminator {
range: Range<usize>,
error_trace: ErrorTrace,
@@ -184,6 +186,7 @@ fn trace_field<T>(res: Result<T>, field_name: &'static str, position: usize) ->
},
)
}
/// Adds a TableField trace detail if `res` is a data error.
fn trace_elem<T>(res: Result<T>, index: usize, position: usize) -> Result<T> {
append_trace(res, ErrorTraceDetail::VectorElement { index, position })
@@ -205,6 +208,7 @@ pub struct VerifierOptions {
// options to error un-recognized enums and unions? possible footgun.
// Ignore nested flatbuffers, etc?
}
impl Default for VerifierOptions {
fn default() -> Self {
Self {
@@ -226,6 +230,7 @@ pub struct Verifier<'opts, 'buf> {
num_tables: usize,
apparent_size: usize,
}
impl<'opts, 'buf> Verifier<'opts, 'buf> {
pub fn new(opts: &'opts VerifierOptions, buffer: &'buf [u8]) -> Self {
Self {
@@ -247,9 +252,12 @@ impl<'opts, 'buf> Verifier<'opts, 'buf> {
/// memory since `buffer: &[u8]` has alignment 1.
///
/// ### WARNING
///
/// This does not work for flatbuffers-structs as they have alignment 1 according to
/// `core::mem::align_of` but are meant to have higher alignment within a Flatbuffer w.r.t.
/// `buffer[0]`. TODO(caspern).
///
/// Note this does not impact soundness as this crate does not assume alignment of structs
#[inline]
fn is_aligned<T>(&self, pos: usize) -> Result<()> {
if pos % core::mem::align_of::<T>() == 0 {
@@ -307,9 +315,9 @@ impl<'opts, 'buf> Verifier<'opts, 'buf> {
// signed offsets are subtracted.
let derefed = if offset > 0 {
pos.checked_sub(offset.abs() as usize)
pos.checked_sub(offset.unsigned_abs() as usize)
} else {
pos.checked_add(offset.abs() as usize)
pos.checked_add(offset.unsigned_abs() as usize)
};
if let Some(x) = derefed {
if x < self.buffer.len() {
@@ -372,6 +380,7 @@ pub struct TableVerifier<'ver, 'opts, 'buf> {
// Verifier struct which holds the surrounding state and options.
verifier: &'ver mut Verifier<'opts, 'buf>,
}
impl<'ver, 'opts, 'buf> TableVerifier<'ver, 'opts, 'buf> {
fn deref(&mut self, field: VOffsetT) -> Result<Option<usize>> {
let field = field as usize;
@@ -439,7 +448,9 @@ impl<'ver, 'opts, 'buf> TableVerifier<'ver, 'opts, 'buf> {
}
(Some(k), Some(v)) => {
trace_field(Key::run_verifier(self.verifier, k), key_field_name, k)?;
let discriminant = Key::follow(self.verifier.buffer, k);
// Safety:
// Run verifier on `k` above
let discriminant = unsafe { Key::follow(self.verifier.buffer, k) };
trace_field(
verify_union(discriminant, self.verifier, v),
val_field_name,
@@ -486,16 +497,27 @@ fn verify_vector_range<T>(v: &mut Verifier, pos: usize) -> Result<core::ops::Ran
}
pub trait SimpleToVerifyInSlice {}
impl SimpleToVerifyInSlice for bool {}
impl SimpleToVerifyInSlice for i8 {}
impl SimpleToVerifyInSlice for u8 {}
impl SimpleToVerifyInSlice for i16 {}
impl SimpleToVerifyInSlice for u16 {}
impl SimpleToVerifyInSlice for i32 {}
impl SimpleToVerifyInSlice for u32 {}
impl SimpleToVerifyInSlice for f32 {}
impl SimpleToVerifyInSlice for i64 {}
impl SimpleToVerifyInSlice for u64 {}
impl SimpleToVerifyInSlice for f64 {}
impl<T: SimpleToVerifyInSlice> Verifiable for Vector<'_, T> {