1297808 - Latent (?) overflow in RTCPPacketInformation::AddApplicationData() could cause buffer overrun

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P2)

Description

[q1]

RTCPPacketInformation::AddApplicationData() (media\webrtc\trunk\webrtc\modules\rtp_rtcp\source\rtcp_receiver_help.cc) can experience an integer overflow. If it does, it allocates a buffer too short for the data that it writes into it. The data written is attacker-defined.

The bug appears to be latent because it appears that UDP is the only currently-used transport for RTCP packets, and the maximum theoretical size of a UDP packet (65507; see https://support.microsoft.com/en-us/kb/822061 ) is too small to cause the overflow. [1] The bug probably can be manifested if FF were to use TCP for RTCP packets, since then the practical maximum size of a compound RTCP packet would be >> 65536.

The bug is in line 66:

55: void RTCPPacketInformation::AddApplicationData(const uint8_t* data,
56:                                                const uint16_t size) {
57:     uint8_t* oldData = applicationData;
58:     uint16_t oldLength = applicationLength;
59: 
60:     // Don't copy more than kRtcpAppCode_DATA_SIZE bytes.
61:     uint16_t copySize = size;
62:     if (size > kRtcpAppCode_DATA_SIZE) {
63:         copySize = kRtcpAppCode_DATA_SIZE;
64:     }
65: 
66:     applicationLength += copySize;

|applicationLength| is declared as a |uint16_t| in rtcp_receiver_help.h. Thus, a compound RTCP packet containing many sufficiently-large AppItem packets (e.g., 65536/kRtcpAppCode_DATA_SIZE) will cause |applicationLength| to overflow, which will cause line 67:

67:     applicationData = new uint8_t[applicationLength];

to allocate a buffer that is too small to contain the accumulated data. Lines 69-74:

68: 
69:     if (oldData)
70:     {
71:         memcpy(applicationData, oldData, oldLength);
72:         memcpy(applicationData+oldLength, data, copySize);
73:         delete [] oldData;
74:     } else

will then copy the accumlated data, of length e.g., 65536 bytes, into the buffer, overwriting whatever's beyond it in the heap.

75:     {
76:         memcpy(applicationData, data, copySize);
77:     }
78: }

[1] And, in any case, the internet MTU for UDP packets is much smaller than 65507.

Comment 1

[Andrew McCreight [:mccr8]]

Randell, could you look at this? Thanks.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

sec-low, since applications have no API to the webrtc stack to add APP RTCP items.  Also, while we support TURN/TCP for webrtc channels, the spec for the encapsulation of messages in TCP there has a 16-bit length, so it's not possible to receive 65536 or large RTCP packets, so even an attacker crafting responses couldn't provoke it.

Likely this isn't even possible using the upstream code directly unless you're very silly.

p2 only because I don't want it to stay on the security radar forever

Worth correcting upstream to avoid carrying patches locally.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Limit combined size of RTCP APP packets

Comment 4

[q1]

(In reply to Randell Jesup [:jesup] from comment #2)
> [W]hile we support TURN/TCP for webrtc channels, the spec for
> the encapsulation of messages in TCP there has a 16-bit length, so it's not
> possible to receive 65536 or large RTCP packets, so even an attacker
> crafting responses couldn't provoke it.

Hmm, how can I force TCP? (media.peerconnection.ice.tcp == true doesn't seem to help). Where is the limit enforced? I see a limit of 9216 bytes in nr_ice_socket_readable_cb(), but I don't know whether that's the only path through the code that RTCP packets can take. That function seems to support TCP, but it doesn't seem to check and strip the (RFC 4571?) 16-bit length.

> Likely this isn't even possible using the upstream code directly unless
> you're very silly.

But a determined attacker will change the source.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

(In reply to q1 from comment #4)
> (In reply to Randell Jesup [:jesup] from comment #2)
> > [W]hile we support TURN/TCP for webrtc channels, the spec for
> > the encapsulation of messages in TCP there has a 16-bit length, so it's not
> > possible to receive 65536 or large RTCP packets, so even an attacker
> > crafting responses couldn't provoke it.
> 
> Hmm, how can I force TCP? (media.peerconnection.ice.tcp == true doesn't seem
> to help). Where is the limit enforced? I see a limit of 9216 bytes in
> nr_ice_socket_readable_cb(), but I don't know whether that's the only path
> through the code that RTCP packets can take.

That's for DTLS packets I believe (DTLS negotiation and DataChannel packets), not DTLS-SRTP packets (RTP/RTCP packets).

? That function seems to support
> TCP, but it doesn't seem to check and strip the (RFC 4571?) 16-bit length.

See above.  It would have been stripped earlier in the TURN code somewhere down in nICEr; not sure where.
 
> > Likely this isn't even possible using the upstream code directly unless
> > you're very silly.
> 
> But a determined attacker will change the source.

I meant it would be hard to fall victim to this if you had a application with a receiver based on upstream code.  I wasn't referring to what an attacker would use to send to us.

Comment 6

[q1]

(In reply to Randell Jesup [:jesup] from comment #5)
> (In reply to q1 from comment #4)
> > (In reply to Randell Jesup [:jesup] from comment #2)
> > > [W]hile we support TURN/TCP for webrtc channels, the spec for
> > > the encapsulation of messages in TCP there has a 16-bit length, so it's not
> > > possible to receive 65536 or large RTCP packets, so even an attacker
> > > crafting responses couldn't provoke it.
> > 
> > Hmm, how can I force TCP? (media.peerconnection.ice.tcp == true doesn't seem
> > to help). Where is the limit enforced? I see a limit of 9216 bytes in
> > nr_ice_socket_readable_cb(), but I don't know whether that's the only path
> > through the code that RTCP packets can take.
> 
> That's for DTLS packets I believe (DTLS negotiation and DataChannel
> packets), not DTLS-SRTP packets (RTP/RTCP packets).

Hmm, I got there by putting a BP on MediaPipeline::PacketReceived() where it calls MediaPipeline::RtcpPacketReceived(), then looking backwards in the call stack. The packet that got me here looks like an RR packet with RC==1:

  81 c9 00 07 76 7a bb a1 16 e5 91 ef 00 00 00 00
  00 00 20 08 00 00 01 b5 00 00 00 00 00 00 00 00

This packet then made it to WebrtcAudioConduit::ReceivedRTCPPacket(), then to VoENetworkImpl::ReceivedRTCPPacket(), then to Channel::ReceivedRTCPPacket(), then to ModuleRtpRtcpImpl::IncomingRtcpPacket(), then to RTCPReceiver::IncomingRTCPPacket(), which (if the packet had contained an an AppItem packet) would have called RtcpReceiver::HandleAppItem() and thence RTCPPackletInformation::AddApplicationData(). But in any case, this code path does have a 9216 byte packet-size limit, so AddApplicationData() can't overflow on packets coming through it.

 
> ? That function seems to support
> > TCP, but it doesn't seem to check and strip the (RFC 4571?) 16-bit length.
> 
> See above.  It would have been stripped earlier in the TURN code somewhere
> down in nICEr; not sure where.

OK. Looks like in nr_socket_buffered_stun_recvfrom() ?

> > > Likely this isn't even possible using the upstream code directly unless
> > > you're very silly.
> > 
> > But a determined attacker will change the source.
> 
> I meant it would be hard to fall victim to this if you had a application
> with a receiver based on upstream code.  I wasn't referring to what an
> attacker would use to send to us.

Got it.

Do you know how I can force webrtc to use TCP?

Comment 7

[Nils Ohlmeier [:drno]]

Agreed there is a potential buffer overflow triggered through an RTCP packet send by the other side to Firefox.

Also Randel is right this should be fixed upstream as well - webrtc.org master still uses exactly the same code.

Now how could an attacker can get such an oversized RTCP packet to Firefox:
1) Via ICE UDP as has been pointed out here already is limited below the overflow because of UDP header size limitations.
2) ICE TCP uses RFC 4571 framing which limits the max packet size itself to 65536. Minus the overhead for the initial RTCP headers that does not leave enough room for the overflow.
3) TURN TCP does not use 4571 framing and therefore could potentially deliver bigger packets. But the input side of the TURN server is always UDP and thus falls under the same limitation as #1. And a rogue TURN server should not be able to break the e2e encryption to insert such a crafted packet.

All three of these paths go through the nr_ice_socket_readable_cb() callback which limits a packet to 9216 bytes read from the wire.

So I don't see how this could actually get abused - except if the attacker manages to create a packet which fits in the 9216 byte limit, but makes the RTCP parser believe if has to read more data.

To answer the open questions:
- Yes nr_socket_buffered_stun_recvfrom() is the function which handles incoming packets on ICE TCP and TURN TCP connection.
- There is no simple way to just force ICE TCP. You first have to set the media.peerconnection.ice.tcp to true. Then restart Firefox. And then you will either have to block UDP with a firewall (not possible with a test page which uses two PeerConnection to connect directly within on browser tab), or your onIceCandidate callback will have to allow only ICE TCP candidates through to the other side. Note: ICE TCP is not yet working when e10s a.k.a. Multiprocess is turned on.

Comment 8

[q1]

@comment 7: Thank you for the comprehensive insights.

Comment 9

[Nils Ohlmeier [:drno]]

I have been thinking about this a little bit more.

Even though the code in mtransport always reads a maximum of 9216 bytes from the wire, that is only the maximum for a single TCP packet. So on a TCP connection you could send a RTCP packet larger then this packet size limit to Firefox.

But with the restrictions explained in comment 7 I see only one way how to leverage this for an attack. The attacker would have to build a pieces of software which acts as TURN server towards Firefox which only accepts TCP connections. The same piece of software would also act as the other end of the PeerConnection. Once the ICE and DTLS have succeeded the attacker could then craft a RTCP packet bigger then 65k bytes, encrypt it with the keys for the PeerConnection and then send it in multiple TCP packets through the TURN TCP connection to the browser.

Comment 10

[q1]

(In reply to Nils Ohlmeier [:drno] from comment #9)
> I have been thinking about this a little bit more....
> 
> Even though the code in mtransport always reads a maximum of 9216 bytes from
> the wire, that is only the maximum for a single TCP packet. So on a TCP
> connection you could send a RTCP packet larger then this packet size limit
> to Firefox.
> 
> But with the restrictions explained in comment 7 I see only one way how to
> leverage this for an attack. The attacker would have to build a pieces of
> software which acts as TURN server towards Firefox which only accepts TCP
> connections. The same piece of software would also act as the other end of
> the PeerConnection. Once the ICE and DTLS have succeeded the attacker could
> then craft a RTCP packet bigger then 65k bytes, encrypt it with the keys for
> the PeerConnection and then send it in multiple TCP packets through the TURN
> TCP connection to the browser.

So an attacker would have to get a user to use her bad TURN server. This appears to be possible according to https://wiki.mozilla.org/Media/WebRTC/Privacy ; if media.peerconnection.use_document_iceservers == true (the default), a page can specify a TURN server. This is a whole new area for me, so I'm unsure of what an attacker might be able to do with this.

Comment 11

[Nils Ohlmeier [:drno]]

(In reply to q1 from comment #10)
> So an attacker would have to get a user to use her bad TURN server. This
> appears to be possible according to
> https://wiki.mozilla.org/Media/WebRTC/Privacy ; if
> media.peerconnection.use_document_iceservers == true (the default), a page
> can specify a TURN server. This is a whole new area for me, so I'm unsure of
> what an attacker might be able to do with this.

Yes and no. What I was trying to describe in comment 9 is highly specific piece of software, something which acts as a TURN server and a PeerConnection at the same time. I doubt that such software exists, because it would be pretty useless outside of this potential attack here - but obviously that doesn't mean it doesn't exist, or can't be build. Just to make it clear that it requires more then "just a bad TURN server".
The attacker would have to run that pieces of software somewhere reachable on the public Internet. And then get the victims browser to visit his page, to connect the victims PeerConnection to his TURN-PeerConnection software by pointing the Firefox's PeerConnection TURN client to his implementation.

Comment 12

[Byron Campen [:bwc]]

(In reply to Nils Ohlmeier [:drno] from comment #7)
> 3) TURN TCP does not use 4571 framing and therefore could potentially
> deliver bigger packets. But the input side of the TURN server is always UDP
> and thus falls under the same limitation as #1. And a rogue TURN server
> should not be able to break the e2e encryption to insert such a crafted
> packet.

While that is true, everything coming from the TURN server is going to be in a DATA indication or a ChannelData message (we don't actually use channels right now). Both of these have a 16 bit length field, so we're still going to have that 65K limit, right?

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0b45e0de73a

Comment 14

[Nils Ohlmeier [:drno]]

(In reply to Byron Campen [:bwc] from comment #12)
> While that is true, everything coming from the TURN server is going to be in
> a DATA indication or a ChannelData message (we don't actually use channels
> right now). Both of these have a 16 bit length field, so we're still going
> to have that 65K limit, right?

Yes you are right. I forgot about that. So that leaves us then only with a packet which is less then 65K on the wire, but makes the parser somehow believe it's bigger then 65k.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a0b45e0de73a