On 7/28/2017 10:47 AM, Michael Niedermayer wrote:
> Fixes: out of array accesses
> Fixes: crash-9238fa9e8d4fde3beda1f279626f53812cb001cb-SEGV
>
> Found-by: JunDong Xie of Ant-financial Light-Year Security Lab
> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> ---
> libavformat/rtmppkt.c | 60
> ++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 20 deletions(-)
>
> diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c
> index 833a3dbade..752d92a42b 100644
> --- a/libavformat/rtmppkt.c
> +++ b/libavformat/rtmppkt.c
> @@ -433,50 +433,70 @@ void ff_rtmp_packet_destroy(RTMPPacket *pkt)
> pkt->size = 0;
> }
>
> -int ff_amf_tag_size(const uint8_t *data, const uint8_t *data_end)
> +static int ff_amf_tag_skip(GetByteContext *gb)
It's static, so no ff_ prefix.
> {
> - const uint8_t *base = data;
> AMFDataType type;
> unsigned nb = -1;
> int parse_key = 1;
>
> - if (data >= data_end)
> + if (bytestream2_get_bytes_left(gb) < 1)
> return -1;
> - switch ((type = *data++)) {
> - case AMF_DATA_TYPE_NUMBER: return 9;
> - case AMF_DATA_TYPE_BOOL: return 2;
> - case AMF_DATA_TYPE_STRING: return 3 + AV_RB16(data);
> - case AMF_DATA_TYPE_LONG_STRING: return 5 + AV_RB32(data);
> - case AMF_DATA_TYPE_NULL: return 1;
> - case AMF_DATA_TYPE_DATE: return 11;
> +
> + switch ((type = bytestream2_get_byte(gb))) {
type = bytestream2_get_byte(gb);
switch(type) {
> + case AMF_DATA_TYPE_NUMBER: bytestream2_get_be64(gb); return 0;
> + case AMF_DATA_TYPE_BOOL: bytestream2_get_byte(gb); return 0;
> + case AMF_DATA_TYPE_STRING:
> + bytestream2_skip(gb, bytestream2_get_be16(gb));
> + return 0;
> + case AMF_DATA_TYPE_LONG_STRING:
> + bytestream2_skip(gb, bytestream2_get_be32(gb));
> + return 0;
> + case AMF_DATA_TYPE_NULL: return 0;
> + case AMF_DATA_TYPE_DATE: bytestream2_skip(gb, 10); return 0;
Use separate lines for all the above or reorder them, for readability's
sake.
> case AMF_DATA_TYPE_ARRAY:
> parse_key = 0;
> case AMF_DATA_TYPE_MIXEDARRAY:
> - nb = bytestream_get_be32(&data);
> + nb = bytestream2_get_be32(gb);
> case AMF_DATA_TYPE_OBJECT:
> while (nb-- > 0 || type != AMF_DATA_TYPE_ARRAY) {
> int t;
> if (parse_key) {
> - int size = bytestream_get_be16(&data);
> + int size = bytestream2_get_be16(gb);
> if (!size) {
> - data++;
> + bytestream2_get_byte(gb);
> break;
> }
> - if (size < 0 || size >= data_end - data)
> + if (size < 0 || size >= bytestream2_get_bytes_left(gb))
> return -1;
> - data += size;
> + bytestream2_skip(gb, size);
> }
> - t = ff_amf_tag_size(data, data_end);
> - if (t < 0 || t >= data_end - data)
> + t = ff_amf_tag_skip(gb);
> + if (t < 0 || bytestream2_get_bytes_left(gb) <= 0)
> return -1;
> - data += t;
> }
> - return data - base;
> - case AMF_DATA_TYPE_OBJECT_END: return 1;
> + return 0;
> + case AMF_DATA_TYPE_OBJECT_END: return 0;
> default: return -1;
> }
> }
>
> +int ff_amf_tag_size(const uint8_t *data, const uint8_t *data_end)
> +{
> + GetByteContext gb;
> + int ret;
> +
> + if (data >= data_end)
> + return -1;
> +
> + bytestream2_init(&gb, data, data_end - data);
> +
> + ret = ff_amf_tag_skip(&gb);
> + if (ret < 0 || bytestream2_get_bytes_left(&gb) <= 0)
> + return -1;
> + av_assert0(bytestream2_tell(&gb) >= 0 && bytestream2_tell(&gb) <=
> data_end - data);
> + return bytestream2_tell(&gb);
> +}
> +
> int ff_amf_get_field_value(const uint8_t *data, const uint8_t *data_end,
> const uint8_t *name, uint8_t *dst, int dst_size)
> {
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
