Fix buffer overflow in HMAC validation of STUN messages.

webrtc/p2p/base/stun_unittest.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 225 matching lines @@
   0x00, 0x01, 0x00, 0x03,  // length of 3
   0x21, 0x12, 0xA4, 0x42,  // magic cookie
   '0', '1', '2', '3',      // transaction id
   '4', '5', '6', '7',
   '8', '9', 'a', 'b',
   0x00, 0x20, 0x00, 0x08,  // xor mapped address
   0x00, 0x01, 0x21, 0x1F,
   0x21, 0x12, 0xA4, 0x53,
 };
 
+static const unsigned char kStunMessageWithBadHmacAtEnd[] = {
+  0x00, 0x01, 0x00, 0x14,  // message length exactly 20
+  0x21, 0x12, 0xA4, 0x42,  // magic cookie
+  '0', '1', '2', '3',      // transaction ID
+  '4', '5', '6', '7',
+  '8', '9', 'a', 'b',
+  0x00, 0x08, 0x00, 0x14,  // type=STUN_ATTR_MESSAGE_INTEGRITY, length=20
+  '0', '0', '0', '0',      // We lied, there are only 16 bytes of HMAC.
+  '0', '0', '0', '0',
+  '0', '0', '0', '0',
+  '0', '0', '0', '0',
+};
+
 // RTCP packet, for testing we correctly ignore non stun packet types.
 // V=2, P=false, RC=0, Type=200, Len=6, Sender-SSRC=85, etc
 static const unsigned char kRtcpPacket[] = {
   0x80, 0xc8, 0x00, 0x06, 0x00, 0x00, 0x00, 0x55,
   0xce, 0xa5, 0x18, 0x3a, 0x39, 0xcc, 0x7d, 0x09,
   0x23, 0xed, 0x19, 0x07, 0x00, 0x00, 0x01, 0x56,
   0x00, 0x03, 0x73, 0x50,
 };
 
 // RFC5769 Test Vectors
@@ skipping 931 matching lines @@
       kRfc5769SampleMsgPassword));
   EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
       reinterpret_cast<const char*>(kStunMessageWithExcessLength),
       sizeof(kStunMessageWithExcessLength),
       kRfc5769SampleMsgPassword));
   EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
       reinterpret_cast<const char*>(kStunMessageWithSmallLength),
       sizeof(kStunMessageWithSmallLength),
       kRfc5769SampleMsgPassword));
 
+  // Again, but with the lengths matching what is claimed in the headers.
+  EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+      reinterpret_cast<const char*>(kStunMessageWithZeroLength),
+      kStunHeaderSize + rtc::GetBE16(&kStunMessageWithZeroLength[2]),
+      kRfc5769SampleMsgPassword));
+  EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+      reinterpret_cast<const char*>(kStunMessageWithExcessLength),
+      kStunHeaderSize + rtc::GetBE16(&kStunMessageWithExcessLength[2]),
+      kRfc5769SampleMsgPassword));
+  EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+      reinterpret_cast<const char*>(kStunMessageWithSmallLength),
+      kStunHeaderSize + rtc::GetBE16(&kStunMessageWithSmallLength[2]),
+      kRfc5769SampleMsgPassword));
+
+  // Check that a too-short HMAC doesn't cause buffer overflow.
+  EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+      reinterpret_cast<const char*>(kStunMessageWithBadHmacAtEnd),
+      sizeof(kStunMessageWithBadHmacAtEnd),
+      kRfc5769SampleMsgPassword));
+
   // Test that munging a single bit anywhere in the message causes the
   // message-integrity check to fail, unless it is after the M-I attribute.
   char buf[sizeof(kRfc5769SampleRequest)];
   memcpy(buf, kRfc5769SampleRequest, sizeof(kRfc5769SampleRequest));
   for (size_t i = 0; i < sizeof(buf); ++i) {
     buf[i] ^= 0x01;
     if (i > 0)
       buf[i - 1] ^= 0x01;
     EXPECT_EQ(i >= sizeof(buf) - 8, StunMessage::ValidateMessageIntegrity(
         buf, sizeof(buf), kRfc5769SampleMsgPassword));
@@ skipping 238 matching lines @@
   EXPECT_TRUE(msg2.Write(&out2));
   EXPECT_EQ(size, out2.Length());
   size_t len2 = out2.Length();
   rtc::ByteBufferReader read_buf2(out2);
   std::string outstring2;
   read_buf2.ReadString(&outstring2, len2);
   EXPECT_EQ(0, memcmp(outstring2.c_str(), input, len2));
 }
 
 }  // namespace cricket