Skip to content

Commit

Permalink
🐛 zv: Prevent f32::NAN from causing panic
Browse files Browse the repository at this point in the history
The range check would fail on NAN, INFINITY and NEG_INFINITY even if
they are completely valid things to decode.

This does not promise that an encoded NAN will be bit-wise identical as
the decoded NAN, but if signalling NAN is used that precisely the user
can have a handful of more interesting problems.

As the decoding of an invalid f64=>f32 shouldn't be a panic, but a
proper error, it means that f64_to_be can better be folded into the code
in the deserialiser.

Since this is floating point, we may also have fun rounding errors as an
f64 may be too _small_ to be represented in an f32 and we may thus have
precision loss as well, that seems annoying to deal with so hopefully
whomever notices that in the future will have a good time.

As I looked at the code, it seems usize => u32/u8 will have the same
"crash the program" kind of conversions in them as well.

Fixes: #1145
  • Loading branch information
Spindel committed Dec 2, 2024
1 parent 737b3ef commit aac2ac9
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 7 deletions.
8 changes: 7 additions & 1 deletion zvariant/src/dbus/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,13 @@ impl<'de, 'd, 'sig, 'f, #[cfg(unix)] F: AsFd, #[cfg(not(unix))] F> de::Deseriali
.endian()
.read_f64(self.0.next_const_size_slice::<f64>()?);

visitor.visit_f32(f64_to_f32(v))
if v.is_finite() && v > (f32::MAX as f64) {
return Err(de::Error::invalid_value(
de::Unexpected::Float(v),
&"Too large for f32",
));
}
visitor.visit_f32(v as f32)
}

fn deserialize_str<V>(self, visitor: V) -> Result<V::Value>
Expand Down
22 changes: 22 additions & 0 deletions zvariant/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2017,6 +2017,28 @@ mod tests {
let _: ZVStruct<'_> = encoded.deserialize_for_signature(signature).unwrap().0;
}

#[test]
fn issue_1145() {
// Ensure f32::NAN can be encoded and decoded.
let ctxt = Context::new_dbus(LE, 0);
{
let encoded = to_bytes(ctxt, &f32::NAN).unwrap();
let result: f32 = encoded.deserialize().unwrap().0;
assert!(result.is_nan());
}
// Ensure f32::INFINITY can be encoded and decoded.
{
let encoded = to_bytes(ctxt, &f32::INFINITY).unwrap();
let result: f32 = encoded.deserialize().unwrap().0;
assert!(result.is_infinite());
}
{
let encoded = to_bytes(ctxt, &f32::NEG_INFINITY).unwrap();
let result: f32 = encoded.deserialize().unwrap().0;
assert!(result.is_infinite());
}
}

#[cfg(feature = "ostree-tests")]
#[test]
fn ostree_de() {
Expand Down
6 changes: 0 additions & 6 deletions zvariant/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,6 @@ pub(crate) fn usize_to_u8(value: usize) -> u8 {
value as u8
}

pub(crate) fn f64_to_f32(value: f64) -> f32 {
assert!(value <= (f32::MAX as f64), "{} too large for `f32`", value,);

value as f32
}

/// Slice the given slice of bytes safely and return an error if the slice is too small.
pub(crate) fn subslice<I, T>(input: &[T], index: I) -> Result<&I::Output>
where
Expand Down

0 comments on commit aac2ac9

Please sign in to comment.