On Tue, 7 Aug 2012, Martin Storsjö wrote:
On Thu, 2 Aug 2012, Samuel Pitoiset wrote:
---
libavformat/rtmpproto.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index 1341cd7..c130b2b 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -1033,7 +1033,11 @@ static int handle_invoke(URLContext *s, RTMPPacket
*pkt)
const uint8_t *data_end = pkt->data + pkt->data_size;
int ret;
- //TODO: check for the messages sent for wrong state?
+ if (pkt->data[0] != AMF_DATA_TYPE_STRING) {
+ av_log(s, AV_LOG_ERROR, "No string method found in invoke
packet\n");
+ return AVERROR_INVALIDDATA;
+ }
+
if (!memcmp(pkt->data, "\002\000\006_error", 9)) {
uint8_t tmpstr[256];
Is this necessary? As far as I can see, each of the memcmps below also check
that the first byte is 2, so this check doesn't change anything at all,
except printing a clearer warning if this would happen (I doubt it ever
happens in practice except when dealing with broken servers, and I'm not sure
if we should litter our code base with extra debugging messages just to aid
someone fixing their broken implementations).
If it isn't necessary, as I see it, but just is a convenience/extra warning
(or cleanup for future code), it would be much easier to accept the change if
you say so in the commit message. Now it took me time to try to figure out
why it was needed.
Also, each of the memcmps check data blindly without checking the packet
size. Yes, it's normally just a few bytes so in most cases an overread
won't cause a crash, but you could trigger something bad. So all of the
memcmps need length checks.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
