Updated ASN1Writer Methods to Use Safe Methods on TLVReader. (project…

…-chip#9532)

-- Replaced unsafe GetLength() and GetBytes() methods on TLVReader with
    safe Get(ByteSpan & v), which performs length/data sanity checks.
 -- Added new ASN1 methods to initialize ASN1Writer and ASN1Reader
    from ByteSpan and arrays.

src/credentials/CHIPCertFromX509.cpp

@@ -711,7 +711,7 @@ CHIP_ERROR ConvertX509CertToChipCert(const ByteSpan x509Cert, MutableByteSpan &
     VerifyOrReturnError(!x509Cert.empty(), CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(CanCastTo<uint32_t>(x509Cert.size()), CHIP_ERROR_INVALID_ARGUMENT);
 
-    reader.Init(x509Cert.data(), static_cast<uint32_t>(x509Cert.size()));
+    reader.Init(x509Cert);
 
     writer.Init(chipCert);
 
@@ -737,8 +737,8 @@ CHIP_ERROR ConvertX509CertsToChipCertArray(const ByteSpan & x509NOC, const ByteS
     ReturnErrorOnFailure(writer.StartContainer(AnonymousTag, kTLVType_Array, outerContainer));
 
     ASN1Reader reader;
-    VerifyOrReturnError(CanCastTo<uint32_t>(x509NOC.size()), CHIP_ERROR_INVALID_ARGUMENT);
-    reader.Init(x509NOC.data(), static_cast<uint32_t>(x509NOC.size()));
+    reader.Init(x509NOC);
+
     uint64_t nocIssuer, nocSubject;
     Optional<uint64_t> nocFabric;
     ReturnErrorOnFailure(ConvertCertificate(reader, writer, AnonymousTag, nocIssuer, nocSubject, nocFabric));

src/credentials/CHIPCertToX509.cpp

@@ -842,7 +842,7 @@ DLL_EXPORT CHIP_ERROR ConvertChipCertToX509Cert(const ByteSpan chipCert, Mutable
 
     reader.Init(chipCert);
 
-    writer.Init(x509Cert.data(), static_cast<uint32_t>(x509Cert.size()));
+    writer.Init(x509Cert);
 
     certData.Clear();
 

src/credentials/GenerateChipX509Cert.cpp

@@ -415,15 +415,14 @@ CHIP_ERROR NewChipX509Cert(const X509CertRequestParams & requestParams, Certific
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     ASN1Writer writer;
-    uint32_t size = static_cast<uint32_t>(std::min(static_cast<size_t>(UINT32_MAX), x509Cert.size()));
-    writer.Init(x509Cert.data(), size);
+    writer.Init(x509Cert);
 
     ReturnErrorOnFailure(EncodeTBSCert(requestParams, issuerLevel, subject, subjectPubkey, issuerKeypair.Pubkey(), writer));
 
     Crypto::P256ECDSASignature signature;
     ReturnErrorOnFailure(issuerKeypair.ECDSA_sign_msg(x509Cert.data(), writer.GetLengthWritten(), signature));
 
-    writer.Init(x509Cert.data(), size);
+    writer.Init(x509Cert);
 
     ASN1_START_SEQUENCE
     {

src/lib/asn1/ASN1.h

@@ -32,6 +32,7 @@
 
 #include <lib/asn1/ASN1Error.h>
 #include <lib/support/DLLUtil.h>
+#include <lib/support/Span.h>
 
 namespace chip {
 namespace TLV {
@@ -101,7 +102,13 @@ struct ASN1UniversalTime
 class DLL_EXPORT ASN1Reader
 {
 public:
-    void Init(const uint8_t * buf, uint32_t len);
+    void Init(const uint8_t * buf, size_t len);
+    void Init(const ByteSpan & data) { Init(data.data(), data.size()); }
+    template <size_t N>
+    void Init(const uint8_t (&data)[N])
+    {
+        Init(data, N);
+    }
 
     uint8_t GetClass(void) const { return Class; };
     uint32_t GetTag(void) const { return Tag; };
@@ -162,7 +169,13 @@ class DLL_EXPORT ASN1Reader
 class DLL_EXPORT ASN1Writer
 {
 public:
-    void Init(uint8_t * buf, uint32_t maxLen);
+    void Init(uint8_t * buf, size_t maxLen);
+    void Init(const MutableByteSpan & data) { Init(data.data(), data.size()); }
+    template <size_t N>
+    void Init(uint8_t (&data)[N])
+    {
+        Init(data, N);
+    }
     void InitNullWriter(void);
     uint16_t GetLengthWritten(void) const;
 
@@ -173,10 +186,10 @@ class DLL_EXPORT ASN1Writer
     CHIP_ERROR PutString(uint32_t tag, const char * val, uint16_t valLen);
     CHIP_ERROR PutOctetString(const uint8_t * val, uint16_t valLen);
     CHIP_ERROR PutOctetString(uint8_t cls, uint32_t tag, const uint8_t * val, uint16_t valLen);
-    CHIP_ERROR PutOctetString(uint8_t cls, uint32_t tag, chip::TLV::TLVReader & val);
+    CHIP_ERROR PutOctetString(uint8_t cls, uint32_t tag, chip::TLV::TLVReader & tlvReader);
     CHIP_ERROR PutBitString(uint32_t val);
     CHIP_ERROR PutBitString(uint8_t unusedBits, const uint8_t * val, uint16_t valLen);
-    CHIP_ERROR PutBitString(uint8_t unusedBits, chip::TLV::TLVReader & val);
+    CHIP_ERROR PutBitString(uint8_t unusedBits, chip::TLV::TLVReader & tlvReader);
     CHIP_ERROR PutTime(const ASN1UniversalTime & val);
     CHIP_ERROR PutNull(void);
     CHIP_ERROR PutConstructedType(const uint8_t * val, uint16_t valLen);
@@ -185,7 +198,7 @@ class DLL_EXPORT ASN1Writer
     CHIP_ERROR StartEncapsulatedType(uint8_t cls, uint32_t tag, bool bitStringEncoding);
     CHIP_ERROR EndEncapsulatedType(void);
     CHIP_ERROR PutValue(uint8_t cls, uint32_t tag, bool isConstructed, const uint8_t * val, uint16_t valLen);
-    CHIP_ERROR PutValue(uint8_t cls, uint32_t tag, bool isConstructed, chip::TLV::TLVReader & val);
+    CHIP_ERROR PutValue(uint8_t cls, uint32_t tag, bool isConstructed, chip::TLV::TLVReader & tlvReader);
 
 private:
     static constexpr size_t kMaxDeferredLengthDepth = kMaxConstructedAndEncapsulatedTypesDepth;
@@ -200,6 +213,7 @@ class DLL_EXPORT ASN1Writer
     CHIP_ERROR WriteDeferredLength(void);
     static uint8_t BytesForLength(int32_t len);
     static void EncodeLength(uint8_t * buf, uint8_t bytesForLen, int32_t lenToEncode);
+    void WriteData(const uint8_t * p, size_t len);
 };
 
 OID ParseObjectID(const uint8_t * encodedOID, uint16_t encodedOIDLen);

src/lib/asn1/ASN1Reader.cpp

@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020 Project CHIP Authors
+ *    Copyright (c) 2020-2021 Project CHIP Authors
  *    Copyright (c) 2013-2017 Nest Labs, Inc.
  *    All rights reserved.
  *
@@ -34,7 +34,7 @@
 namespace chip {
 namespace ASN1 {
 
-void ASN1Reader::Init(const uint8_t * buf, uint32_t len)
+void ASN1Reader::Init(const uint8_t * buf, size_t len)
 {
     ResetElementState();
     mBuf              = buf;

src/lib/asn1/ASN1Writer.cpp

@@ -52,7 +52,7 @@ enum
     kUnknownLengthMarker    = 0xFF
 };
 
-void ASN1Writer::Init(uint8_t * buf, uint32_t maxLen)
+void ASN1Writer::Init(uint8_t * buf, size_t maxLen)
 {
     mBuf                 = buf;
     mWritePoint          = buf;
@@ -125,9 +125,9 @@ CHIP_ERROR ASN1Writer::PutOctetString(uint8_t cls, uint32_t tag, const uint8_t *
     return PutValue(cls, tag, false, val, valLen);
 }
 
-CHIP_ERROR ASN1Writer::PutOctetString(uint8_t cls, uint32_t tag, chip::TLV::TLVReader & val)
+CHIP_ERROR ASN1Writer::PutOctetString(uint8_t cls, uint32_t tag, chip::TLV::TLVReader & tlvReader)
 {
-    return PutValue(cls, tag, false, val);
+    return PutValue(cls, tag, false, tlvReader);
 }
 
 static uint8_t ReverseBits(uint8_t v)
@@ -229,27 +229,27 @@ CHIP_ERROR ASN1Writer::PutBitString(uint8_t unusedBitCount, const uint8_t * enco
 
     *mWritePoint++ = unusedBitCount;
 
-    memcpy(mWritePoint, encodedBits, encodedBitsLen);
-    mWritePoint += encodedBitsLen;
+    WriteData(encodedBits, encodedBitsLen);
 
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR ASN1Writer::PutBitString(uint8_t unusedBitCount, chip::TLV::TLVReader & encodedBits)
+CHIP_ERROR ASN1Writer::PutBitString(uint8_t unusedBitCount, chip::TLV::TLVReader & tlvReader)
 {
-    uint32_t encodedBitsLen;
+    ByteSpan encodedBits;
 
     // Do nothing for a null writer.
     VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
 
-    encodedBitsLen = encodedBits.GetLength();
+    ReturnErrorOnFailure(tlvReader.Get(encodedBits));
 
-    ReturnErrorOnFailure(EncodeHead(kASN1TagClass_Universal, kASN1UniversalTag_BitString, false, encodedBitsLen + 1));
+    VerifyOrReturnError(CanCastTo<int32_t>(encodedBits.size() + 1), ASN1_ERROR_LENGTH_OVERFLOW);
+
+    ReturnErrorOnFailure(EncodeHead(kASN1TagClass_Universal, kASN1UniversalTag_BitString, false, encodedBits.size() + 1));
 
     *mWritePoint++ = unusedBitCount;
 
-    encodedBits.GetBytes(mWritePoint, encodedBitsLen);
-    mWritePoint += encodedBitsLen;
+    WriteData(encodedBits.data(), encodedBits.size());
 
     return CHIP_NO_ERROR;
 }
@@ -298,8 +298,7 @@ CHIP_ERROR ASN1Writer::PutConstructedType(const uint8_t * val, uint16_t valLen)
     // Make sure we have enough space to write
     VerifyOrReturnError((mWritePoint + valLen) <= mBufEnd, ASN1_ERROR_OVERFLOW);
 
-    memcpy(mWritePoint, val, valLen);
-    mWritePoint += valLen;
+    WriteData(val, valLen);
 
     return CHIP_NO_ERROR;
 }
@@ -345,25 +344,25 @@ CHIP_ERROR ASN1Writer::PutValue(uint8_t cls, uint32_t tag, bool isConstructed, c
 
     ReturnErrorOnFailure(EncodeHead(cls, tag, isConstructed, valLen));
 
-    memcpy(mWritePoint, val, valLen);
-    mWritePoint += valLen;
+    WriteData(val, valLen);
 
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR ASN1Writer::PutValue(uint8_t cls, uint32_t tag, bool isConstructed, chip::TLV::TLVReader & val)
+CHIP_ERROR ASN1Writer::PutValue(uint8_t cls, uint32_t tag, bool isConstructed, chip::TLV::TLVReader & tlvReader)
 {
-    uint32_t valLen;
+    ByteSpan val;
 
     // Do nothing for a null writer.
     VerifyOrReturnError(mBuf != nullptr, CHIP_NO_ERROR);
 
-    valLen = val.GetLength();
+    ReturnErrorOnFailure(tlvReader.Get(val));
 
-    ReturnErrorOnFailure(EncodeHead(cls, tag, isConstructed, valLen));
+    VerifyOrReturnError(CanCastTo<int32_t>(val.size()), ASN1_ERROR_LENGTH_OVERFLOW);
+
+    ReturnErrorOnFailure(EncodeHead(cls, tag, isConstructed, val.size()));
 
-    val.GetBytes(mWritePoint, valLen);
-    mWritePoint += valLen;
+    WriteData(val.data(), val.size());
 
     return CHIP_NO_ERROR;
 }
@@ -386,6 +385,7 @@ CHIP_ERROR ASN1Writer::EncodeHead(uint8_t cls, uint32_t tag, bool isConstructed,
     bytesForLen = BytesForLength(len);
 
     // Make sure there's enough space to encode the entire value.
+    // Note that the calculated total length doesn't overflow because `len` is a signed value (int32_t).
     totalLen = 1 + bytesForLen + (len != kUnknownLength ? len : 0);
     VerifyOrReturnError((mWritePoint + totalLen) <= mBufEnd, ASN1_ERROR_OVERFLOW);
 
@@ -496,5 +496,11 @@ void ASN1Writer::EncodeLength(uint8_t * buf, uint8_t bytesForLen, int32_t lenToE
     }
 }
 
+void ASN1Writer::WriteData(const uint8_t * p, size_t len)
+{
+    memcpy(mWritePoint, p, len);
+    mWritePoint += len;
+}
+
 } // namespace ASN1
 } // namespace chip