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? -- Best regards, Samuel Pitoiset. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
