Fix buffer overflow in HMAC validation of STUN messages.

webrtc/p2p/base/stun.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 131 matching lines @@
     return false;
   }
 
   // Finding Message Integrity attribute in stun message.
   size_t current_pos = kStunHeaderSize;
   bool has_message_integrity_attr = false;
   while (current_pos < size) {
     uint16_t attr_type, attr_length;
     // Getting attribute type and length.
     attr_type = rtc::GetBE16(&data[current_pos]);
     attr_length = rtc::GetBE16(&data[current_pos + sizeof(attr_type)]);

[Sergey Ulanov, 2016/06/17 00:52:49]

I think here we need to verify that current_pos + 4 < size

[katrielc1, 2016/06/17 08:21:55]

I think you don't technically need to verify that, since size is guaranteed to
be a multiple of four so if there's anything to read then there's enough.

But it makes sense to add it for robustness, so will do :)

 
     // If M-I, sanity check it, and break out.
     if (attr_type == STUN_ATTR_MESSAGE_INTEGRITY) {
       if (attr_length != kStunMessageIntegritySize ||
-          current_pos + attr_length > size) {
+          current_pos + attr_length >= size) {

[Sergey Ulanov, 2016/06/17 00:52:49]

Should this be "current_pos + sizeof(attr_type) + sizeof(attr_length) +
attr_lenght > size"?

[katrielc1, 2016/06/17 08:21:55]

Yes, you're right.

(I think it may be equivalent since current_pos is rounded to the nearest 4
later, but this is the right thing to check.)

         return false;
       }
       has_message_integrity_attr = true;
       break;
     }
 
     // Otherwise, skip to the next attribute.
     current_pos += sizeof(attr_type) + sizeof(attr_length) + attr_length;
     if ((attr_length % 4) != 0) {
       current_pos += (4 - (attr_length % 4));
@@ skipping 743 matching lines @@
       digest, sizeof(digest));
   if (size == 0) {
     return false;
   }
 
   *hash = std::string(digest, size);
   return true;
 }
 
 }  // namespace cricket