On Thu, 26 Jul 2012, Samuel Pitoiset wrote:
On Thu, Jul 26, 2012 at 8:25 PM, Martin Storsjö <mar...@martin.st> wrote:
On Thu, 26 Jul 2012, Samuel Pitoiset wrote:
On Thu, Jul 26, 2012 at 8:17 PM, Martin Storsjö <mar...@martin.st> wrote:
On Thu, 26 Jul 2012, Samuel Pitoiset wrote:
On Thu, Jul 26, 2012 at 7:27 PM, Martin Storsjö <mar...@martin.st>
wrote:
On Thu, 26 Jul 2012, Samuel Pitoiset wrote:
---
libavformat/rtmpproto.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index a2efe38..a32c4a9 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -915,6 +915,12 @@ static int handle_ping(URLContext *s, RTMPPacket
*pkt)
t = AV_RB16(pkt->data);
if (t == 6) {
+ if (pkt->data_size < 6) {
+ av_log(s, AV_LOG_ERROR, "Too short ping packet (%d)\n",
+ pkt->data_size);
+ return AVERROR_INVALIDDATA;
+ }
+
The commit and warning messages are good this time, however the code
itself
is wrong in two different ways. Where did you get the number 6, and why
do
you do the check here?
I played a stream using a FMS and I used Wireshark in order to find
the size of a ping packet.
I do the check here because that handle function can received other
packets like a swfverification request, for example. And if this
packet is not 6 bytes long a *wrong* error code is returned.
Your argumentation is flawed, and so is your way of figuring out the size
limit.
Why do we check the size of buffers?
We check the size of buffers in order to prevent reading outside an
allocated buffer.
Correct. Now in the handle_ping function, where do we read from the buffer
we got from the network?
Got it! The check should be added before 't = AV_RB16(pkt->data);'
otherwise we could read outside of the buffer...
Yes. And you should check for 2 bytes, not 6.
Your method of checking with wireshark is not good for this, because FMS
might have sent you a 15 byte packet. We might only be reading the first 6
bytes of it, and other servers might send less, so just setting the check
to whatever one particular server happens to send is not good.
This function only reads bytes 0,1 from the packet, and thus you should
make sure size is >= 2. In gen_pong, it reads bytes 2, 3, 4, 5, so there
you should check that size >= 6. It's better to have that check in that
function instead of in handle_ping, since then it is clear why you check
for that particular amount.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
