security(twdata): Zeroize every TWData instance in TWDataDelete, and other TWData improvements

src/interface/TWData.cpp

@@ -4,6 +4,7 @@
 
 #include <TrustWalletCore/TWData.h>
 #include <TrustWalletCore/TWString.h>
+#include <TrezorCrypto/memzero.h>
 #include "Data.h"
 #include "HexCoding.h"
 #include <algorithm>
@@ -49,21 +50,33 @@ uint8_t *_Nonnull TWDataBytes(TWData *_Nonnull data) {
 
 uint8_t TWDataGet(TWData *_Nonnull data, size_t index) {
     auto* v = reinterpret_cast<const Data*>(data);
+    if (v->size() <= index) {
+        throw std::out_of_range("index out of range");
+    }
     return (*v)[index];
 }
 
 void TWDataSet(TWData *_Nonnull data, size_t index, uint8_t byte) {
     auto* v = const_cast<Data*>(reinterpret_cast<const Data*>(data));
+    if (v->size() <= index) {
+        throw std::out_of_range("index out of range");
+    }
     (*v)[index] = byte;
 }
 
 void TWDataCopyBytes(TWData *_Nonnull data, size_t start, size_t size, uint8_t *_Nonnull output) {
     auto* v = reinterpret_cast<const Data*>(data);
+    if (start + size > v->size()) {
+        throw std::out_of_range("index out of range");
+    }
     std::copy(std::begin(*v) + start, std::begin(*v) + start + size, output);
 }
 
 void TWDataReplaceBytes(TWData *_Nonnull data, size_t start, size_t size, const uint8_t *_Nonnull bytes) {
     auto* v = const_cast<Data*>(reinterpret_cast<const Data*>(data));
+    if (start + size > v->size()) {
+            throw std::out_of_range("index out of range");
+    }
     std::copy(bytes, bytes + size, std::begin(*v) + start);
 }
 
@@ -95,8 +108,12 @@ void TWDataReset(TWData *_Nonnull data) {
 }
 
 void TWDataDelete(TWData *_Nonnull data) {
-    auto* v = reinterpret_cast<const Data*>(data);
-    delete v;
+    auto* vConst = reinterpret_cast<const Data*>(data);
+    // `const_cast` is safe here despite that the pointer to the data is const
+    // but `Data` is not a constant value.
+    auto *v = const_cast<Data*>(vConst);
+    memzero(v->data(), v->size());
+    delete vConst;
 }
 
 bool TWDataEqual(TWData *_Nonnull lhs, TWData *_Nonnull rhs) {

tests/interface/TWBase32Tests.cpp

@@ -11,7 +11,7 @@
 
 TEST(TWBase32, InvalidDecode) {
     const auto encodedInput = STRING("JBSWY3DPK5XXE3DE=======");
-    auto result = WRAPD(TWBase32Decode(encodedInput.get()));
+    auto result = TWBase32Decode(encodedInput.get());
     ASSERT_EQ(result, nullptr);
 }
 

tests/interface/TWBase58Tests.cpp

@@ -40,8 +40,8 @@ TEST(TWBase58, DecodeNoCheck) {
 
 TEST(TWBase58, Decode_WrongChecksum) {
     const auto input = STRING("1Bp9U1ogV3A14FMvKbRJms7ctyso5FdSz2");
-    auto result = WRAPD(TWBase58Decode(input.get()));
-    ASSERT_EQ(result.get(), nullptr);
+    auto result = TWBase58Decode(input.get());
+    ASSERT_EQ(result, nullptr);
 }
 
 TEST(TWBase58, DecodeNoCheck_WrongChecksum) {
@@ -54,13 +54,13 @@ TEST(TWBase58, DecodeNoCheck_WrongChecksum) {
 TEST(TWBase58, Decode_InvalidChar) {
     // 0 is invalid
     const auto input = STRING("1Bp9U1ogV3A14FMvKbRJms7ctyso4Z4Tc0");
-    auto result = WRAPD(TWBase58Decode(input.get()));
-    ASSERT_EQ(result.get(), nullptr);
+    auto result = TWBase58Decode(input.get());
+    ASSERT_EQ(result, nullptr);
 }
 
 TEST(TWBase58, DecodeNoCheck_InvalidChar) {
     // 0 is invalid
     const auto input = STRING("1Bp9U1ogV3A14FMvKbRJms7ctyso4Z4Tc0");
-    auto result = WRAPD(TWBase58DecodeNoCheck(input.get()));
-    ASSERT_EQ(result.get(), nullptr);
+    auto result = TWBase58DecodeNoCheck(input.get());
+    ASSERT_EQ(result, nullptr);
 }

tests/interface/TWBech32Tests.cpp

@@ -24,8 +24,8 @@ TEST(TWBech32, Decode) {
 TEST(TWBech32, Decode_WrongChecksumVariant) {
     // This is a Bech32m variant, not Bech32 variant. So it should fail using Bech32 decoder.
     const auto input = STRING("abcdef1l7aum6echk45nj3s0wdvt2fg8x9yrzpqzd3ryx");
-    const auto result = WRAPD(TWBech32Decode(input.get()));
-    ASSERT_EQ(result.get(), nullptr);
+    const auto result = TWBech32Decode(input.get());
+    ASSERT_EQ(result, nullptr);
 }
 
 TEST(TWBech32, EncodeM) {
@@ -44,6 +44,6 @@ TEST(TWBech32, DecodeM) {
 TEST(TWBech32, DecodeM_WrongChecksumVariant) {
     // This is a Bech32 variant, not Bech32m variant. So it should fail using Bech32M decoder.
     const auto input = STRING("abcdef1qpzry9x8gf2tvdw0s3jn54khce6mua7lmqqqxw");
-    const auto result = WRAPD(TWBech32DecodeM(input.get()));
-    ASSERT_EQ(result.get(), nullptr);
+    const auto result = TWBech32DecodeM(input.get());
+    ASSERT_EQ(result, nullptr);
 }

tests/interface/TWDataTests.cpp

@@ -46,8 +46,8 @@ TEST(TWData, CreateWithHexString) {
     {
         // null input
         TWString* nullstring = nullptr;
-        const auto data = WRAPD(TWDataCreateWithHexString(nullstring));
-        ASSERT_EQ(data.get(), nullptr);
+        const auto data = TWDataCreateWithHexString(nullstring);
+        ASSERT_EQ(data, nullptr);
     }
 }