Fix casting in TLVReader::Get to be safe.

Casts of out-of-range values to signed integer types are not defined
by the C or C++ standards, but this code was expecting them to act as
a cast to the corresponding-sized unsigned integer type, then the
"undoing" of a signed-to-unsigned cast.

We instead introduce a CastToSigned() function that explicitly and
safely undoes a signed-to-unsigned cast and use that in TLVReader::Get
to eliminate the implementation-defined behavior dependence.

src/lib/core/CHIPTLVReader.cpp

@@ -28,6 +28,7 @@
 #include <core/CHIPEncoding.h>
 #include <core/CHIPTLV.h>
 #include <support/CodeUtils.h>
+#include <support/SafeInt.h>
 #include <system/SystemPacketBuffer.h>
 
 namespace chip {
@@ -370,7 +371,7 @@ CHIP_ERROR TLVReader::Get(int8_t & v)
 {
     uint64_t v64   = 0;
     CHIP_ERROR err = Get(v64);
-    v              = v64;
+    v              = CastToSigned(static_cast<uint8_t>(v64));
     return err;
 }
 
@@ -391,7 +392,7 @@ CHIP_ERROR TLVReader::Get(int16_t & v)
 {
     uint64_t v64   = 0;
     CHIP_ERROR err = Get(v64);
-    v              = v64;
+    v              = CastToSigned(static_cast<uint16_t>(v64));
     return err;
 }
 
@@ -412,7 +413,7 @@ CHIP_ERROR TLVReader::Get(int32_t & v)
 {
     uint64_t v64   = 0;
     CHIP_ERROR err = Get(v64);
-    v              = v64;
+    v              = CastToSigned(static_cast<uint32_t>(v64));
     return err;
 }
 
@@ -433,7 +434,7 @@ CHIP_ERROR TLVReader::Get(int64_t & v)
 {
     uint64_t v64   = 0;
     CHIP_ERROR err = Get(v64);
-    v              = v64;
+    v              = CastToSigned(v64);
     return err;
 }
 
@@ -520,13 +521,13 @@ CHIP_ERROR TLVReader::Get(uint64_t & v)
     switch (ElementType())
     {
     case kTLVElementType_Int8:
-        v = static_cast<int64_t>(static_cast<int8_t>(mElemLenOrVal));
+        v = static_cast<uint64_t>(static_cast<int64_t>(CastToSigned(static_cast<uint8_t>(mElemLenOrVal))));
         break;
     case kTLVElementType_Int16:
-        v = static_cast<int64_t>(static_cast<int16_t>(mElemLenOrVal));
+        v = static_cast<uint64_t>(static_cast<int64_t>(CastToSigned(static_cast<uint16_t>(mElemLenOrVal))));
         break;
     case kTLVElementType_Int32:
-        v = static_cast<int64_t>(static_cast<int32_t>(mElemLenOrVal));
+        v = static_cast<uint64_t>(static_cast<int64_t>(CastToSigned(static_cast<uint32_t>(mElemLenOrVal))));
         break;
     case kTLVElementType_Int64:
     case kTLVElementType_UInt8:

src/lib/support/SafeInt.h

@@ -105,6 +105,38 @@ bool CanCastTo(U arg)
     return 0 <= arg && static_cast<uintmax_t>(arg) <= static_cast<uintmax_t>(numeric_limits<T>::max());
 }
 
+/**
+ * A function to reverse the effects of a signed-to-unsigned integer cast.
+ *
+ * If the argument is small enough to be representable as a positive signed
+ * integer, returns that integer.  Otherwise, returns a negative integer which
+ * would, if cast to the type of the argument, produce the given value.
+ *
+ * So for example, if a uint8_t with value 254 is passed in this function will
+ * return an int8_t with value -2.
+ */
+template <typename T>
+typename std::enable_if<std::is_unsigned<T>::value, typename std::make_signed<T>::type>::type CastToSigned(T arg)
+{
+    using namespace std;
+    typedef typename make_signed<T>::type signed_type;
+
+    if (arg <= static_cast<T>(numeric_limits<signed_type>::max()))
+    {
+        return static_cast<signed_type>(arg);
+    }
+
+    // We want to return arg - (numeric_limits<T>::max() + 1), but do it without
+    // hitting overflow.  We do this by rewriting it as:
+    //
+    // -(numeric_limits<T>::max() - arg) - 1
+    //
+    // then noting that both (numeric_limits<T>::max() - arg) and its negation
+    // are guaranteed to fit in signed_type.
+    signed_type diff = static_cast<signed_type>(numeric_limits<T>::max() - arg);
+    return -diff - 1;
+}
+
 } // namespace chip
 
 #endif // SAFEINT_H_