On Mon, Dec 28, 2015 at 05:29:35AM +0000, Eran Kornblau wrote: > Bumping up this thread (latest patch attached) > > Happy holidays !
happy holidays as well!
>
> Eran
>
[...]
> @@ -4007,6 +4009,162 @@ static int mov_read_free(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> return 0;
> }
>
> +static int mov_read_frma(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> +{
> + uint32_t format = avio_rl32(pb);
> + MOVStreamContext *sc;
> + enum AVCodecID id;
> + AVStream *st;
> +
> + if (c->fc->nb_streams < 1)
> + return 0;
> + st = c->fc->streams[c->fc->nb_streams - 1];
> + sc = st->priv_data;
> +
> + switch (sc->format)
> + {
> + case MKTAG('e','n','c','v'): // encrypted video
> + case MKTAG('e','n','c','a'): // encrypted audio
> + id = mov_codec_id(st, format);
> + st->codec->codec_id = id;
this seems missing a check for st->codec->codec_id being "unset"
before setting it
> + sc->format = format;
> + break;
> +
> + default:
> + av_log(c->fc, AV_LOG_WARNING,
> + "ignoring 'frma' atom of '%.4s', stream format is '%.4s'\n",
> + (char*)&format, (char*)&sc->format);
the way these are printed would lead to different results on big and
little endian
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int mov_read_senc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> +{
> + AVStream *st;
> + MOVStreamContext *sc;
> + size_t auxiliary_info_size;
> + int ret;
> +
> + if (c->decryption_key_len == 0 || c->fc->nb_streams < 1)
> + return 0;
> +
> + st = c->fc->streams[c->fc->nb_streams - 1];
> + sc = st->priv_data;
> +
> + if (sc->cenc.aes_ctr) {
> + av_log(c->fc, AV_LOG_ERROR, "duplicate senc atom\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + avio_r8(pb); /* version */
> + sc->cenc.use_subsamples = avio_rb24(pb) & 0x02; /* flags */
> +
> + avio_rb32(pb); /* entries */
> +
> + if (atom.size < 8) {
> + av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" too small\n",
> atom.size);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + /* save the auxiliary info as is */
> + auxiliary_info_size = atom.size - 8;
> +
> + sc->cenc.auxiliary_info = av_malloc(auxiliary_info_size);
> + if (!sc->cenc.auxiliary_info) {
> + return AVERROR(ENOMEM);
> + }
> +
> + sc->cenc.auxiliary_info_end = sc->cenc.auxiliary_info +
> auxiliary_info_size;
> +
> + sc->cenc.auxiliary_info_pos = sc->cenc.auxiliary_info;
> +
> + if (avio_read(pb, sc->cenc.auxiliary_info, auxiliary_info_size) !=
> auxiliary_info_size) {
> + av_log(c->fc, AV_LOG_ERROR, "failed to read the auxiliary info");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + /* initialize the cipher */
> + sc->cenc.aes_ctr = av_aes_ctr_alloc();
> + if (!sc->cenc.aes_ctr) {
> + return AVERROR(ENOMEM);
> + }
> +
> + ret = av_aes_ctr_init(sc->cenc.aes_ctr, c->decryption_key);
> + if (ret) {
> + return ret;
> + }
> +
> + return 0;
this could be simplfied
also errors in ffmpeg are generally negative so
if (ret < 0) seems more correct if a seperate check is left
> +}
> +
> +static int cenc_filter(MOVContext *c, MOVStreamContext *sc, uint8_t *input,
> int size)
> +{
> + uint32_t encrypted_bytes;
> + uint16_t subsample_count;
> + uint16_t clear_bytes;
> + uint8_t* input_end = input + size;
> +
> + /* read the iv */
> + if (sc->cenc.auxiliary_info_pos + AES_CTR_IV_SIZE >
> sc->cenc.auxiliary_info_end) {
> + av_log(c->fc, AV_LOG_ERROR, "failed to read iv from the auxiliary
> info\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + av_aes_ctr_set_iv(sc->cenc.aes_ctr, sc->cenc.auxiliary_info_pos);
> + sc->cenc.auxiliary_info_pos += AES_CTR_IV_SIZE;
> +
> + if (!sc->cenc.use_subsamples)
> + {
> + /* decrypt the whole packet */
> + av_aes_ctr_crypt(sc->cenc.aes_ctr, input, input, size);
> + return 0;
> + }
> +
> + /* read the subsample count */
> + if (sc->cenc.auxiliary_info_pos + sizeof(uint16_t) >
> sc->cenc.auxiliary_info_end) {
the addition could overflow, which would be undefined behavior
something like:
sizeof(uint16_t) > sc->cenc.auxiliary_info_end - sc->cenc.auxiliary_info_pos
is correcter
> + av_log(c->fc, AV_LOG_ERROR, "failed to read subsample count from the
> auxiliary info\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + subsample_count = AV_RB16(sc->cenc.auxiliary_info_pos);
> + sc->cenc.auxiliary_info_pos += sizeof(uint16_t);
> +
> + for (; subsample_count > 0; subsample_count--)
> + {
> + if (sc->cenc.auxiliary_info_pos + 6 > sc->cenc.auxiliary_info_end) {
> + av_log(c->fc, AV_LOG_ERROR, "failed to read subsample from the
> auxiliary info\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + /* read the number of clear / encrypted bytes */
> + clear_bytes = AV_RB16(sc->cenc.auxiliary_info_pos);
> + sc->cenc.auxiliary_info_pos += sizeof(uint16_t);
> + encrypted_bytes = AV_RB32(sc->cenc.auxiliary_info_pos);
> + sc->cenc.auxiliary_info_pos += sizeof(uint32_t);
> +
> + if (input + clear_bytes + encrypted_bytes > input_end) {
the additions can overflow
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No great genius has ever existed without some touch of madness. -- Aristotle
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
