Index: content/browser/renderer_host/p2p/socket_host.cc |
diff --git a/content/browser/renderer_host/p2p/socket_host.cc b/content/browser/renderer_host/p2p/socket_host.cc |
index fd26298c1b7c3397558aa9cfef2dce874aefdffd..9a80fd097f8c0043a4057be09470dc108e5f009a 100644 |
--- a/content/browser/renderer_host/p2p/socket_host.cc |
+++ b/content/browser/renderer_host/p2p/socket_host.cc |
@@ -33,8 +33,8 @@ static const unsigned char kFakeAuthTag[10] = { |
0xba, 0xdd, 0xba, 0xdd, 0xba, 0xdd, 0xba, 0xdd, 0xba, 0xdd |
}; |
-bool IsTurnChannelData(const char* data) { |
- return ((*data & 0xC0) == 0x40); |
+bool IsTurnChannelData(const char* data, int len) { |
palmer
2014/09/22 19:34:46
size_t, or explicitly-sized unsigned type.
http:/
jiayl
2014/09/22 21:39:02
Done.
|
+ return len >= 1 && ((*data & 0xC0) == 0x40); |
palmer
2014/09/22 19:34:46
Is there a maximum sane size we should check for,
jiayl
2014/09/22 21:39:03
I don't think there is a maximum.
juberti2
2014/09/22 22:43:56
We have a max packet size of 2048 that we use else
jiayl
2014/09/22 23:04:13
Done.
|
} |
bool IsDtlsPacket(const char* data, int len) { |
@@ -47,7 +47,10 @@ bool IsRtcpPacket(const char* data) { |
return (type >= 64 && type < 96); |
} |
-bool IsTurnSendIndicationPacket(const char* data) { |
+bool IsTurnSendIndicationPacket(const char* data, int len) { |
+ if (len < 2) |
palmer
2014/09/22 19:34:46
I'd suggest using kMinimumWhateverLength and kMaxi
jiayl
2014/09/22 21:39:03
Used the minimum header length for these methods.
|
+ return false; |
+ |
uint16 type = rtc::GetBE16(data); |
return (type == cricket::TURN_SEND_INDICATION); |
} |
@@ -78,6 +81,10 @@ bool ValidateRtpHeader(const char* rtp, int length, size_t* header_length) { |
rtp += rtp_hdr_len_without_extn; |
+ if (rtp_hdr_len_without_extn + kRtpExtnHdrLen > length) { |
palmer
2014/09/22 19:34:46
Integer overflow potential? Consider using checked
jiayl
2014/09/22 21:39:02
The max of the sum is 12 + 4 * 31 + 4 * UINT16_MAX
|
+ return false; |
+ } |
+ |
// Getting extension profile length. |
// Length is in 32 bit words. |
uint16 extn_length = rtc::GetBE16(rtp + 2) * 4; |
aedla
2014/09/22 20:06:47
Would be nice to fix the integer overflow here.
jiayl
2014/09/22 21:39:03
Changed the result to size_t to avoid overflow.
|
@@ -109,7 +116,11 @@ void UpdateAbsSendTimeExtnValue(char* extn_data, int len, |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |
// | ID | len=2 | absolute send time | |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |
- DCHECK_EQ(len, kAbsSendTimeExtnLen); |
+ if (len != kAbsSendTimeExtnLen) { |
+ NOTREACHED(); |
+ return; |
+ } |
+ |
// Now() has resolution ~1-15ms, using HighResNow(). But it is warned not to |
// use it unless necessary, as it is expensive than Now(). |
uint32 now_second = abs_send_time; |
@@ -148,15 +159,17 @@ void UpdateRtpAuthTag(char* rtp, int len, |
return; |
} |
- if (hmac.DigestLength() < tag_length) { |
+ const size_t kRocLength = 4; |
+ |
+ if (tag_length > hmac.DigestLength() || tag_length < kRocLength) { |
aedla
2014/09/22 20:06:47
That won't cut it. Say tag_length == hmac.DigestLe
jiayl
2014/09/22 21:39:03
Done.
|
NOTREACHED(); |
return; |
} |
// Copy ROC after end of rtp packet. |
- memcpy(auth_tag, &options.packet_time_params.srtp_packet_index, 4); |
+ memcpy(auth_tag, &options.packet_time_params.srtp_packet_index, kRocLength); |
// Authentication of a RTP packet will have RTP packet + ROC size. |
- int auth_required_length = len - tag_length + 4; |
+ int auth_required_length = len - tag_length + kRocLength; |
palmer
2014/09/22 19:34:46
Integer overflow potential?
jiayl
2014/09/22 21:39:03
tag_length is guaranteed to be >= kRocLength, so n
|
unsigned char output[64]; |
if (!hmac.Sign(base::StringPiece(rtp, auth_required_length), |
@@ -221,9 +234,12 @@ bool GetRtpPacketStartPositionAndLength(const char* packet, |
int length, |
int* rtp_start_pos, |
int* rtp_packet_length) { |
+ *rtp_start_pos = 0; |
+ *rtp_packet_length = 0; |
+ |
int rtp_begin; |
int rtp_length = 0; |
- if (IsTurnChannelData(packet)) { |
+ if (IsTurnChannelData(packet, length)) { |
// Turn Channel Message header format. |
// 0 1 2 3 |
// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 |
@@ -243,7 +259,7 @@ bool GetRtpPacketStartPositionAndLength(const char* packet, |
if (length < rtp_length + kTurnChannelHdrLen) { |
return false; |
} |
- } else if (IsTurnSendIndicationPacket(packet)) { |
+ } else if (IsTurnSendIndicationPacket(packet, length)) { |
if (length <= P2PSocketHost::kStunHeaderSize) { |
// Message must be greater than 20 bytes, if it's carrying any payload. |
return false; |
@@ -274,12 +290,19 @@ bool GetRtpPacketStartPositionAndLength(const char* packet, |
// padding so that its value contains a multiple of 4 bytes. The |
// padding bits are ignored, and may be any value. |
uint16 attr_type, attr_length; |
+ const int kAttrHeaderLength = sizeof(attr_type) + sizeof(attr_length); |
+ |
+ if (length < rtp_begin + kAttrHeaderLength) { |
+ return false; |
+ } |
+ |
// Getting attribute type and length. |
attr_type = rtc::GetBE16(&packet[rtp_begin]); |
attr_length = rtc::GetBE16( |
&packet[rtp_begin + sizeof(attr_type)]); |
+ |
// Checking for bogus attribute length. |
- if (length < attr_length + rtp_begin) { |
+ if (length < rtp_begin + kAttrHeaderLength + attr_length) { |
return false; |
} |
@@ -380,10 +403,13 @@ bool UpdateRtpAbsSendTimeExtn(char* rtp, int length, |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |
// | data | |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |
- char* extn_start = rtp; |
+ const char* extn_start = rtp; |
aedla
2014/09/22 20:06:46
Having
const char* extn_end = extn_start + extn_le
jiayl
2014/09/22 21:39:03
Done.
|
while (rtp - extn_start < extn_length) { |
const int id = (*rtp & 0xF0) >> 4; |
const int len = (*rtp & 0x0F) + 1; |
+ if (rtp + kOneByteHdrLen + len - extn_start >= extn_length) { |
aedla
2014/09/22 20:06:48
I think this should be > extn_length. Or
if (rtp +
jiayl
2014/09/22 21:39:03
You are right. Done.
|
+ return false; |
+ } |
// The 4-bit length is the number minus one of data bytes of this header |
// extension element following the one-byte header. |
if (id == extension_id) { |
@@ -393,7 +419,7 @@ bool UpdateRtpAbsSendTimeExtn(char* rtp, int length, |
} |
rtp += kOneByteHdrLen + len; |
// Counting padding bytes. |
- while ((*rtp == 0) && (rtp - extn_start < extn_length)) { |
+ while ((rtp - extn_start < extn_length) && (*rtp == 0)) { |
++rtp; |
} |
} |