On Thu, 26 Jul 2012, Samuel Pitoiset wrote:

On Thu, Jul 26, 2012 at 8:31 PM, Samuel Pitoiset
<[email protected]> wrote:
On Thu, Jul 26, 2012 at 8:25 PM, Martin Storsjö <[email protected]> wrote:
On Thu, 26 Jul 2012, Samuel Pitoiset wrote:

On Thu, Jul 26, 2012 at 8:17 PM, Martin Storsjö <[email protected]> wrote:

On Thu, 26 Jul 2012, Samuel Pitoiset wrote:

On Thu, Jul 26, 2012 at 7:27 PM, Martin Storsjö <[email protected]>
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...

Wrong! We have to check if the buffer is equal or more than 2 bytes
long before extracting the type...
Do you agree?

Exactly.

There's no weird magic around, and you don't need to use wireshark or any extra tools for figuring it out. If you read element [x] in an array, you need to make sure that x < len, before you read it. Always. Nothing more, nothing less.

// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to