From aac2ac970c4a47622f2c99810daba17b8beac416 Mon Sep 17 00:00:00 2001
From: "D.S. Ljungmark" <spider@skuggor.se>
Date: Sun, 17 Nov 2024 19:36:36 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20zv:=20Prevent=20f32::NAN=20from?=
 =?UTF-8?q?=20causing=20panic?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/dbus2/zbus/issues/1145
---
 zvariant/src/dbus/de.rs |  8 +++++++-
 zvariant/src/lib.rs     | 22 ++++++++++++++++++++++
 zvariant/src/utils.rs   |  6 ------
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/zvariant/src/dbus/de.rs b/zvariant/src/dbus/de.rs
index c57abbdb2..00aed6b77 100644
--- a/zvariant/src/dbus/de.rs
+++ b/zvariant/src/dbus/de.rs
@@ -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>
diff --git a/zvariant/src/lib.rs b/zvariant/src/lib.rs
index 8bac8e418..e1a648958 100644
--- a/zvariant/src/lib.rs
+++ b/zvariant/src/lib.rs
@@ -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() {
diff --git a/zvariant/src/utils.rs b/zvariant/src/utils.rs
index 3c6cb60a5..f3a4ac2b1 100644
--- a/zvariant/src/utils.rs
+++ b/zvariant/src/utils.rs
@@ -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