[FFmpeg-devel] [PATCH] avformat: Add Pro-MPEG CoP #3-R2 FEC protocol

[Michael Niedermayer]

On Mon, May 30, 2016 at 11:04:03AM +0200, Vlad Tarca wrote:
[...]
> +static int prompeg_init(URLContext *h, const uint8_t *buf, int size) {
> +    PrompegContext *s = h->priv_data;
> +    uint32_t seed;
> +    int recovery_len, bitstring_len, rtp_buf_len;
> +    int i;
> +
> +    s->fec_buf = NULL;
> +    s->rtp_buf = NULL;
> +
> +    s->packet_idx = 0;
> +    s->packet_idx_max = s->l * s->d;
> +    s->packet_size = size;
> +

> +    recovery_len = size - 12;

can size be INT_MIN ?
if so this can overflow and nothing after this line can correct this
the compiler has every right to crash you here or do anything else
signed overflow is undefined in C, unsigned is defined


> +    if (recovery_len <= 0) {
> +        av_log(h, AV_LOG_ERROR, "Invalid recovery length\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +    s->recovery_len = recovery_len;
> +
> +    rtp_buf_len = 12 + 16 + s->recovery_len;

same issue here


> +    if (rtp_buf_len <= 0 || rtp_buf_len >= INT_MAX / sizeof (uint8_t)) {

sizeof (uint8_t) is 1

rtp_buf_len can never be larger than INT_MAX as its a int


[...]
> +static int prompeg_close(URLContext *h) {
> +    PrompegContext *s = h->priv_data;
> +    int i;
> +
> +    ffurl_close(s->fec_col_hd);
> +    ffurl_close(s->fec_row_hd);

ffurl_closep() also clears the pointer


[...]
> @@ -561,6 +616,9 @@ static int rtp_close(URLContext *h)
>  
>      ffurl_close(s->rtp_hd);
>      ffurl_close(s->rtcp_hd);

> +    if (s->fec) {
> +        ffurl_close(s->fec_hd);
> +    }

why does this need a check ?
isnt fec_hd NULL if it shouldnt be closed ?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160530/204cefbe/attachment.sig>