-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix crashes found hunting the last open fuzzing record #8524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f24691e
40e67e1
40a839b
bb744d6
7e4b96e
6e47646
2432fd7
02a6193
ff52fe0
d7cf971
61b989b
c9449a6
ee782e0
fab2b3b
e3dcd8e
773ae23
125d5aa
62ed618
c65fe42
00b64c3
7c40cb9
ce43008
3d69690
366ef3e
ab0d4ff
7fad3b0
a7f7021
691d6ca
3139b95
dce42a3
5a5ba17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,14 @@ pub enum MarshalError { | |||||
| InvalidLocation, | ||||||
| /// Bad type marker | ||||||
| BadType, | ||||||
| /// A type marker no reader knows | ||||||
| UnknownType, | ||||||
| /// A back reference that names nothing | ||||||
| InvalidRef, | ||||||
| /// A marker that stands for no object at all | ||||||
| NullObject, | ||||||
| /// A container length that is negative or does not fit, named by what it counts | ||||||
| BadSize(&'static str), | ||||||
| } | ||||||
|
|
||||||
| impl core::fmt::Display for MarshalError { | ||||||
|
|
@@ -29,6 +37,10 @@ impl core::fmt::Display for MarshalError { | |||||
| Self::InvalidUtf8 => f.write_str("invalid utf8"), | ||||||
| Self::InvalidLocation => f.write_str("invalid source location"), | ||||||
| Self::BadType => f.write_str("bad type marker"), | ||||||
| Self::UnknownType => f.write_str("unknown type code"), | ||||||
| Self::InvalidRef => f.write_str("invalid reference"), | ||||||
| Self::NullObject => f.write_str("NULL object in marshal data for object"), | ||||||
| Self::BadSize(what) => write!(f, "{what} size out of range"), | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -111,7 +123,7 @@ impl TryFrom<u8> for Type { | |||||
| b'A' => Self::AsciiInterned, | ||||||
| b'z' => Self::ShortAscii, | ||||||
| b'Z' => Self::ShortAsciiInterned, | ||||||
| _ => return Err(MarshalError::BadType), | ||||||
| _ => return Err(MarshalError::UnknownType), | ||||||
| }) | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -146,6 +158,13 @@ pub trait Read { | |||||
| fn read_u64(&mut self) -> Result<u64> { | ||||||
| Ok(u64::from_le_bytes(*self.read_array()?)) | ||||||
| } | ||||||
|
|
||||||
| /// A length, read the way `r_long` reads one: it is signed, so a value | ||||||
| /// with the top bit set is out of range rather than four billion items. | ||||||
| fn read_len(&mut self, what: &'static str) -> Result<usize> { | ||||||
| let len = self.read_u32()? as i32; | ||||||
| usize::try_from(len).map_err(|_| MarshalError::BadSize(what)) | ||||||
| } | ||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||
| } | ||||||
|
|
||||||
| pub(crate) trait ReadBorrowed<'a>: Read { | ||||||
|
|
@@ -305,7 +324,7 @@ fn reserve_ref_slot<T>(has_flag: bool, refs: &mut Vec<Option<T>>) -> Option<usiz | |||||
| fn resolve_ref<T: Clone>(idx: usize, refs: &[Option<T>]) -> Result<T> { | ||||||
| refs.get(idx) | ||||||
| .and_then(|v| v.clone()) | ||||||
| .ok_or(MarshalError::InvalidBytecode) | ||||||
| .ok_or(MarshalError::InvalidRef) | ||||||
| } | ||||||
|
|
||||||
| /// Read a marshal bytes object (TYPE_STRING = b's'), resolving TYPE_REF | ||||||
|
|
@@ -408,7 +427,7 @@ fn read_marshal_str_vec<R: Read, Bag: ConstantBag>( | |||||
| } | ||||||
|
|
||||||
| let n = match type_byte { | ||||||
| b'(' => rdr.read_u32()? as usize, | ||||||
| b'(' => rdr.read_len("tuple")?, | ||||||
| b')' => rdr.read_u8()? as usize, | ||||||
| _ => return Err(MarshalError::BadType), | ||||||
| }; | ||||||
|
|
@@ -471,7 +490,7 @@ fn read_marshal_const_tuple<R: Read, Bag: ConstantBag>( | |||||
| } | ||||||
|
|
||||||
| let n = match type_byte { | ||||||
| b'(' => rdr.read_u32()? as usize, | ||||||
| b'(' => rdr.read_len("tuple")?, | ||||||
| b')' => rdr.read_u8()? as usize, | ||||||
| _ => return Err(MarshalError::BadType), | ||||||
| }; | ||||||
|
|
@@ -553,7 +572,7 @@ pub trait MarshalBag: Copy { | |||||
| fn make_code( | ||||||
| &self, | ||||||
| code: CodeObject<<Self::ConstantBag as ConstantBag>::Constant>, | ||||||
| ) -> Self::Value; | ||||||
| ) -> Result<Self::Value>; | ||||||
|
|
||||||
| /// Construct a runtime code object while retaining the exact values read | ||||||
| /// from ``co_consts``. Compiler bags ignore this second channel; runtime | ||||||
|
|
@@ -563,7 +582,7 @@ pub trait MarshalBag: Copy { | |||||
| &self, | ||||||
| code: CodeObject<<Self::ConstantBag as ConstantBag>::Constant>, | ||||||
| _constants: Vec<Self::Value>, | ||||||
| ) -> Self::Value { | ||||||
| ) -> Result<Self::Value> { | ||||||
| self.make_code(code) | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -583,8 +602,12 @@ pub trait MarshalBag: Copy { | |||||
| /// Install partially-built containers in the marshal reference table | ||||||
| /// before reading their children, as CPython's `r_object()` does. | ||||||
| /// Runtime bags can opt in; constant bags retain collect-then-construct. | ||||||
| fn make_tuple_placeholder(&self, _len: usize) -> Option<Self::Value> { | ||||||
| None | ||||||
| /// | ||||||
| /// `len` comes straight from the input and is only bounded by what a | ||||||
| /// length can hold, so a bag that opts in reports the room it cannot get | ||||||
| /// rather than taking it for granted. | ||||||
| fn make_tuple_placeholder(&self, _len: usize) -> Result<Option<Self::Value>> { | ||||||
| Ok(None) | ||||||
| } | ||||||
|
|
||||||
| fn set_tuple_item( | ||||||
|
|
@@ -596,8 +619,8 @@ pub trait MarshalBag: Copy { | |||||
| Err(MarshalError::BadType) | ||||||
| } | ||||||
|
|
||||||
| fn make_list_placeholder(&self, _len: usize) -> Option<Self::Value> { | ||||||
| None | ||||||
| fn make_list_placeholder(&self, _len: usize) -> Result<Option<Self::Value>> { | ||||||
| Ok(None) | ||||||
| } | ||||||
|
|
||||||
| fn set_list_item(&self, _list: &Self::Value, _index: usize, _value: Self::Value) -> Result<()> { | ||||||
|
|
@@ -725,8 +748,8 @@ impl<Bag: ConstantBag> MarshalBag for Bag { | |||||
| fn make_code( | ||||||
| &self, | ||||||
| code: CodeObject<<Self::ConstantBag as ConstantBag>::Constant>, | ||||||
| ) -> Self::Value { | ||||||
| self.make_code(code) | ||||||
| ) -> Result<Self::Value> { | ||||||
| Ok(self.make_code(code)) | ||||||
| } | ||||||
|
|
||||||
| fn make_stop_iter(&self) -> Result<Self::Value> { | ||||||
|
|
@@ -830,10 +853,7 @@ fn deserialize_value_after_header<R: Read, Bag: MarshalBag>( | |||||
| // TYPE_REF: return previously stored object | ||||||
| if type_code == Type::Ref as u8 { | ||||||
| let idx = rdr.read_u32()? as usize; | ||||||
| return refs | ||||||
| .get(idx) | ||||||
| .and_then(|v| v.clone()) | ||||||
| .ok_or(MarshalError::InvalidBytecode); | ||||||
| return resolve_ref(idx, refs); | ||||||
| } | ||||||
|
|
||||||
| // Reserve ref slot before reading (matches write order) | ||||||
|
|
@@ -986,7 +1006,7 @@ fn deserialize_code_value_inner<R: Read, Bag: MarshalBag>( | |||||
| linetable, | ||||||
| exceptiontable, | ||||||
| }; | ||||||
| Ok(bag.make_code_with_constants(code, constant_values)) | ||||||
| bag.make_code_with_constants(code, constant_values) | ||||||
| } | ||||||
|
|
||||||
| fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | ||||||
|
|
@@ -1033,13 +1053,13 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | |||||
| bag.make_complex(value) | ||||||
| } | ||||||
| Type::Ascii | Type::Unicode => { | ||||||
| let len = rdr.read_u32()?; | ||||||
| let value = rdr.read_wtf8(len)?; | ||||||
| let len = rdr.read_len("string")?; | ||||||
| let value = rdr.read_wtf8(len as u32)?; | ||||||
| bag.make_str(value) | ||||||
| } | ||||||
| Type::AsciiInterned | Type::Interned => { | ||||||
| let len = rdr.read_u32()?; | ||||||
| let value = rdr.read_wtf8(len)?; | ||||||
| let len = rdr.read_len("string")?; | ||||||
| let value = rdr.read_wtf8(len as u32)?; | ||||||
| bag.make_interned_str(value) | ||||||
| } | ||||||
| Type::ShortAscii => { | ||||||
|
|
@@ -1056,7 +1076,7 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | |||||
| let len = rdr.read_u8()? as usize; | ||||||
| let d = depth - 1; | ||||||
| if let Some(index) = slot | ||||||
| && let Some(tuple) = bag.make_tuple_placeholder(len) | ||||||
| && let Some(tuple) = bag.make_tuple_placeholder(len)? | ||||||
| { | ||||||
| refs[index] = Some(tuple.clone()); | ||||||
| for item_index in 0..len { | ||||||
|
|
@@ -1070,17 +1090,17 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | |||||
| } | ||||||
| } | ||||||
| Type::Null => { | ||||||
| return Err(MarshalError::BadType); | ||||||
| return Err(MarshalError::NullObject); | ||||||
| } | ||||||
| Type::Ref => { | ||||||
| // Handled in deserialize_value_depth before calling this function | ||||||
| return Err(MarshalError::BadType); | ||||||
| } | ||||||
| Type::Tuple => { | ||||||
| let len = rdr.read_u32()? as usize; | ||||||
| let len = rdr.read_len("tuple")?; | ||||||
| let d = depth - 1; | ||||||
| if let Some(index) = slot | ||||||
| && let Some(tuple) = bag.make_tuple_placeholder(len) | ||||||
| && let Some(tuple) = bag.make_tuple_placeholder(len)? | ||||||
| { | ||||||
| refs[index] = Some(tuple.clone()); | ||||||
| for item_index in 0..len { | ||||||
|
|
@@ -1094,10 +1114,10 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | |||||
| } | ||||||
| } | ||||||
| Type::List => { | ||||||
| let len = rdr.read_u32()? as usize; | ||||||
| let len = rdr.read_len("list")?; | ||||||
| let d = depth - 1; | ||||||
| if let Some(index) = slot | ||||||
| && let Some(list) = bag.make_list_placeholder(len) | ||||||
| && let Some(list) = bag.make_list_placeholder(len)? | ||||||
| { | ||||||
| refs[index] = Some(list.clone()); | ||||||
| for item_index in 0..len { | ||||||
|
|
@@ -1111,7 +1131,7 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | |||||
| } | ||||||
| } | ||||||
| Type::Set => { | ||||||
| let len = rdr.read_u32()? as usize; | ||||||
| let len = rdr.read_len("set")?; | ||||||
| let d = depth - 1; | ||||||
| if let Some(index) = slot | ||||||
| && let Some(set) = bag.make_set_placeholder() | ||||||
|
|
@@ -1128,7 +1148,7 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | |||||
| } | ||||||
| } | ||||||
| Type::FrozenSet => { | ||||||
| let len = rdr.read_u32()?; | ||||||
| let len = rdr.read_len("set")?; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Use the correct container name in the size error. At Line 1141, Proposed fix- let len = rdr.read_len("set")?;
+ let len = rdr.read_len("frozenset")?;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| let d = depth - 1; | ||||||
| let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); | ||||||
| itertools::process_results(it, |it| bag.make_frozenset(it))?? | ||||||
|
|
@@ -1165,8 +1185,8 @@ fn deserialize_value_typed<R: Read, Bag: MarshalBag>( | |||||
| } | ||||||
| Type::Bytes => { | ||||||
| // After marshaling, byte arrays are converted into bytes. | ||||||
| let len = rdr.read_u32()?; | ||||||
| let value = rdr.read_slice(len)?; | ||||||
| let len = rdr.read_len("bytes object")?; | ||||||
| let value = rdr.read_slice(len as u32)?; | ||||||
| bag.make_bytes(value) | ||||||
| } | ||||||
| Type::Code => return Err(MarshalError::BadType), | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.