On Mon, Jun 4, 2018 at 10:46 AM Carl Eugen Hoyos <ceffm...@gmail.com> wrote: > > 2018-06-04 18:59 GMT+02:00, Jacob Trimble <modmaker-at-google....@ffmpeg.org>: > > On Fri, Jun 1, 2018 at 5:03 PM Michael Niedermayer > > <mich...@niedermayer.cc> wrote: > >> > >> On Thu, May 31, 2018 at 09:33:36AM -0700, Jacob Trimble wrote: > >> > Found by Chrome's ClusterFuzz: http://crbug.com/846662. > >> > > >> > Signed-off-by: Jacob Trimble <modma...@google.com> > >> > --- > >> > libavutil/encryption_info.c | 7 +++++-- > >> > 1 file changed, 5 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c > >> > index 20a752d6b4..a48ded922c 100644 > >> > --- a/libavutil/encryption_info.c > >> > +++ b/libavutil/encryption_info.c > >> > @@ -64,6 +64,8 @@ AVEncryptionInfo *av_encryption_info_clone(const > >> > AVEncryptionInfo *info) > >> > { > >> > AVEncryptionInfo *ret; > >> > > >> > + if (!info) > >> > + return NULL; > >> > ret = av_encryption_info_alloc(info->subsample_count, > >> > info->key_id_size, info->iv_size); > >> > if (!ret) > >> > return NULL; > >> > >> > @@ -127,7 +129,7 @@ uint8_t *av_encryption_info_add_side_data(const > >> > AVEncryptionInfo *info, size_t * > >> > uint8_t *buffer, *cur_buffer; > >> > uint32_t i; > >> > > >> > - if (UINT32_MAX - FF_ENCRYPTION_INFO_EXTRA < info->key_id_size || > >> > + if (!info || !size || UINT32_MAX - FF_ENCRYPTION_INFO_EXTRA < > >> > info->key_id_size || > >> > UINT32_MAX - FF_ENCRYPTION_INFO_EXTRA - info->key_id_size < > >> > info->iv_size || > >> > (UINT32_MAX - FF_ENCRYPTION_INFO_EXTRA - info->key_id_size - > >> > info->iv_size) / 8 < info->subsample_count) { > >> > return NULL; > >> > @@ -260,7 +262,8 @@ uint8_t *av_encryption_init_info_add_side_data(const > >> > AVEncryptionInitInfo *info, > >> > uint8_t *buffer, *cur_buffer; > >> > uint32_t i, max_size; > >> > > >> > - if (UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA < > >> > info->system_id_size || > >> > + if (!info || !side_data_size || > >> > + UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA < > >> > info->system_id_size || > >> > UINT32_MAX - FF_ENCRYPTION_INIT_INFO_EXTRA - > >> > info->system_id_size < info->data_size) { > >> > return NULL; > >> > } > >> > >> in which valid case would these be called with NULL input ? > >> iam asking as this feels as if it might be a bug in teh caller > >> > > > > This was found by Chrome's ClusterFuzz, which I am not that familiar > > with. I think it was just running fuzz tests directly on FFmpeg code, > > so it wasn't in production code. But since this is a public method, > > we should validate the input in any case. > > How do you validate the size of C buffers in general? >
I'm not sure I understand your comment. You can't verify the length of buffers unless the size is given to the method. These functions do accept the size and verify that the data is valid for the given size. Since we are verifying the data and the size we are given, we should be checking for NULL as well. > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
