From 58fa1bac0392f58f5060237fbbf8060dafe0f744 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Thu, 8 Apr 2021 16:31:47 +0200 Subject: [PATCH] dcsctp: Enforce variable length TLV minimum length The length field was validated to not be too big, or to have too much padding, but it could be smaller than the fixed size of the chunk, which isn't correct. Now it's enforced to be at minimum the size of the fixed size header. Bug: webrtc:12614 Change-Id: I57089a5ba2854eeb63ab3b4e28cf5878087d06e8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214484 Reviewed-by: Tommi Commit-Queue: Victor Boivie Cr-Commit-Position: refs/heads/master@{#33659} --- net/dcsctp/packet/tlv_trait.h | 2 +- net/dcsctp/packet/tlv_trait_test.cc | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/net/dcsctp/packet/tlv_trait.h b/net/dcsctp/packet/tlv_trait.h index 11c3852d38..7e2e58d601 100644 --- a/net/dcsctp/packet/tlv_trait.h +++ b/net/dcsctp/packet/tlv_trait.h @@ -105,7 +105,7 @@ class TLVTrait { } } else { // Expect variable length data - verify its size alignment. - if (length > data.size()) { + if (length > data.size() || length < Config::kHeaderSize) { tlv_trait_impl::ReportInvalidVariableLengthField(length, data.size()); return absl::nullopt; } diff --git a/net/dcsctp/packet/tlv_trait_test.cc b/net/dcsctp/packet/tlv_trait_test.cc index 413c71e452..a0dd1a1136 100644 --- a/net/dcsctp/packet/tlv_trait_test.cc +++ b/net/dcsctp/packet/tlv_trait_test.cc @@ -77,7 +77,7 @@ struct TwoByteTypeConfig { static constexpr int kTypeSizeInBytes = 2; static constexpr int kType = 31337; static constexpr size_t kHeaderSize = 8; - static constexpr int kVariableLengthAlignment = 4; + static constexpr int kVariableLengthAlignment = 2; }; class TwoByteChunk : public TLVTrait { @@ -122,5 +122,12 @@ TEST(TlvDataTest, CanReadTwoByteTypeTlvs) { ElementsAre(0x05, 0x06, 0x07, 0x08, 0xDE, 0xAD, 0xBE, 0xEF)); } +TEST(TlvDataTest, CanHandleInvalidLengthSmallerThanFixedSize) { + // Has 'length=6', which is below the kHeaderSize of 8. + uint8_t data[] = {0x7A, 0x69, 0x00, 0x06, 0x01, 0x02, 0x03, 0x04}; + + EXPECT_FALSE(TwoByteChunk::Parse(data).has_value()); +} + } // namespace } // namespace dcsctp