TLV reader: Fix float support and remove union UB (#8531)

* TLV reader: Improve floating-point support

1. For some reason the Get(float&) signature was declared in the .h file
   but was never defined. Define it as well. Disallow reading a 64-bit
   floating point number to float, to avoid silent truncation / loss of
   precision.
2. It is undefined behaviour to use unions for type-punning in C++. Use
   raw memcpy calls instead, which is defined by the C++ standard
   (std::bit_cast is not available).

   Compilers will generally be able to optimize this memcpy away, unless
   a freestanding implementation (with possibly non-standard memcpy) is
   used.

* TLV writer: Remove union UB

It is undefined behaviour to use unions for type-punning in C++. Use
raw memcpy calls instead, as a substitute for the unavailable
std::bit_cast.

src/lib/core/CHIPTLVReader.cpp

@@ -207,28 +207,46 @@ CHIP_ERROR TLVReader::Get(uint64_t & v)
     return CHIP_NO_ERROR;
 }
 
+namespace {
+float BitCastToFloat(const uint64_t elemLenOrVal)
+{
+    float f;
+    auto u32 = static_cast<uint32_t>(elemLenOrVal);
+    memcpy(&f, &u32, sizeof(f));
+    return f;
+}
+} // namespace
+
+// Note: Unlike the integer Get functions, this code avoids doing conversions
+// between float and double wherever possible, because these conversions are
+// relatively expensive on platforms that use soft-float instruction sets.
+
+CHIP_ERROR TLVReader::Get(float & v)
+{
+    switch (ElementType())
+    {
+    case TLVElementType::FloatingPointNumber32: {
+        v = BitCastToFloat(mElemLenOrVal);
+        break;
+    }
+    default:
+        return CHIP_ERROR_WRONG_TLV_TYPE;
+    }
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR TLVReader::Get(double & v)
 {
     switch (ElementType())
     {
     case TLVElementType::FloatingPointNumber32: {
-        union
-        {
-            uint32_t u32;
-            float f;
-        } cvt;
-        cvt.u32 = static_cast<uint32_t>(mElemLenOrVal);
-        v       = cvt.f;
+        v = BitCastToFloat(mElemLenOrVal);
         break;
     }
     case TLVElementType::FloatingPointNumber64: {
-        union
-        {
-            uint64_t u64;
-            double d;
-        } cvt;
-        cvt.u64 = mElemLenOrVal;
-        v       = cvt.d;
+        double d;
+        memcpy(&d, &mElemLenOrVal, sizeof(d));
+        v = d;
         break;
     }
     default:

src/lib/core/CHIPTLVWriter.cpp

@@ -211,26 +211,18 @@ CHIP_ERROR TLVWriter::Put(uint64_t tag, int64_t v, bool preserveSize)
     return Put(tag, v);
 }
 
-CHIP_ERROR TLVWriter::Put(uint64_t tag, float v)
+CHIP_ERROR TLVWriter::Put(uint64_t tag, const float v)
 {
-    union
-    {
-        float f;
-        uint32_t u32;
-    } cvt;
-    cvt.f = v;
-    return WriteElementHead(TLVElementType::FloatingPointNumber32, tag, cvt.u32);
+    uint32_t u32;
+    memcpy(&u32, &v, sizeof(u32));
+    return WriteElementHead(TLVElementType::FloatingPointNumber32, tag, u32);
 }
 
-CHIP_ERROR TLVWriter::Put(uint64_t tag, double v)
+CHIP_ERROR TLVWriter::Put(uint64_t tag, const double v)
 {
-    union
-    {
-        double d;
-        uint64_t u64;
-    } cvt;
-    cvt.d = v;
-    return WriteElementHead(TLVElementType::FloatingPointNumber64, tag, cvt.u64);
+    uint64_t u64;
+    memcpy(&u64, &v, sizeof(u64));
+    return WriteElementHead(TLVElementType::FloatingPointNumber64, tag, u64);
 }
 
 CHIP_ERROR TLVWriter::Put(uint64_t tag, ByteSpan data)

src/lib/core/tests/TestCHIPTLV.cpp

@@ -582,8 +582,8 @@ void ReadEncoding1(nlTestSuite * inSuite, TLVReader & reader)
 
         TestNext<TLVReader>(inSuite, reader2);
 
-        TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535),
-                                   static_cast<float>(17.9));
+        TestGet<TLVReader, float>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);
+        TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);
 
         TestNext<TLVReader>(inSuite, reader2);
 
@@ -2965,8 +2965,8 @@ void TestCHIPTLVReaderDup(nlTestSuite * inSuite)
 
         TestNext<TLVReader>(inSuite, reader2);
 
-        TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535),
-                                   static_cast<float>(17.9));
+        TestGet<TLVReader, float>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);
+        TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);
 
         TestNext<TLVReader>(inSuite, reader2);
 
@@ -2994,6 +2994,11 @@ void TestCHIPTLVReaderErrorHandling(nlTestSuite * inSuite)
     err = reader.Get(val);
     NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_WRONG_TLV_TYPE);
 
+    // Get(float&)
+    float numF;
+    err = reader.Get(numF);
+    NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_WRONG_TLV_TYPE);
+
     // Get(double&)
     double numD;
     err = reader.Get(numD);
@@ -3040,6 +3045,38 @@ void TestCHIPTLVReaderErrorHandling(nlTestSuite * inSuite)
     NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_WRONG_TLV_TYPE);
     chip::Platform::MemoryFree(const_cast<uint8_t *>(data));
 }
+
+/**
+ *  Test that CHIP TLV reader returns an error when a read is requested that
+ *  would truncate the output.
+ */
+void TestCHIPTLVReaderTruncatedReads(nlTestSuite * inSuite)
+{
+    uint8_t buf[2048];
+    TLVWriter writer;
+    TLVReader reader;
+
+    CHIP_ERROR err;
+    float outF;
+
+    // Setup: Write some values into the buffer
+    writer.Init(buf, sizeof(buf));
+    writer.ImplicitProfileId = TestProfile_2;
+
+    err = writer.Put(AnonymousTag, double{ 12.5 });
+    NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+
+    // Test reading values from the buffer
+    reader.Init(buf, sizeof(buf));
+
+    TestNext<TLVReader>(inSuite, reader);
+
+    TestGet<TLVReader, double>(inSuite, reader, kTLVType_FloatingPointNumber, AnonymousTag, 12.5);
+
+    err = reader.Get(outF);
+    NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_WRONG_TLV_TYPE);
+}
+
 /**
  *  Test CHIP TLV Reader in a use case
  */
@@ -3068,8 +3105,8 @@ void TestCHIPTLVReaderInPractice(nlTestSuite * inSuite)
 
     TestNext<TLVReader>(inSuite, reader);
 
-    TestGet<TLVReader, double>(inSuite, reader, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_1, 4000000000ULL),
-                               static_cast<float>(1.0));
+    TestGet<TLVReader, float>(inSuite, reader, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_1, 4000000000ULL),
+                              static_cast<float>(1.0));
 }
 
 void TestCHIPTLVReader_NextOverContainer_ProcessElement(nlTestSuite * inSuite, TLVReader & reader, void * context)
@@ -3170,6 +3207,8 @@ void CheckCHIPTLVReader(nlTestSuite * inSuite, void * inContext)
 
     TestCHIPTLVReaderErrorHandling(inSuite);
 
+    TestCHIPTLVReaderTruncatedReads(inSuite);
+
     TestCHIPTLVReaderInPractice(inSuite);
 
     TestCHIPTLVReader_NextOverContainer(inSuite);