On 2/28/2017 3:21 PM, Vittorio Giovara wrote: > On Tue, Feb 28, 2017 at 1:15 PM, James Almer <jamr...@gmail.com> wrote: >> On 2/28/2017 2:46 PM, Vittorio Giovara wrote: >>> On Tue, Feb 28, 2017 at 11:00 AM, James Almer <jamr...@gmail.com> wrote: >>>> On 2/22/2017 1:21 PM, James Almer wrote: >>>>> On 2/21/2017 7:35 PM, Vittorio Giovara wrote: >>>> >>>> CCing this one as well. >>>> >>>>>> --- >>>>>> libavformat/matroskadec.c | 64 >>>>>> ++++++++++++++++++++++++++++++++-- >>>>>> tests/ref/fate/matroska-spherical-mono | 6 +++- >>>>>> 2 files changed, 66 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >>>>>> index 7223e94..0a02237 100644 >>>>>> --- a/libavformat/matroskadec.c >>>>>> +++ b/libavformat/matroskadec.c >>>>>> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream >>>>>> *st, const MatroskaTrack *track) >>>>>> AVSphericalMapping *spherical; >>>>>> enum AVSphericalProjection projection; >>>>>> size_t spherical_size; >>>>>> + size_t l, t, r, b; >>>>>> + size_t padding = 0; >>>>>> int ret; >>>>>> + GetByteContext gb; >>>>>> + >>>>>> + bytestream2_init(&gb, track->video.projection.private.data, >>>>>> + track->video.projection.private.size); >>>>> >>>>> private.size can be zero and private.data NULL. bytestream2_init() >>>>> will trigger an assert in those cases. >>> >>> :( asserts like this are really dampening, it should be on read not on >>> creation (if at all) >> >> True, guess it should return an error value instead. > > For now what should we do? Just check that private.size and > private.data are non-0 and return with an error otherwise?
Actually, nevermind. The assert check is for size >= 0, so it's just checking for negative values (probably fuzzing related). Sorry for the noise. The code should be ok, as you're using the checked versions of the read functions. > >>>>>> + if (bytestream2_get_byte(&gb) != 0) { >>>>>> + av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n"); >>>>>> + return 0; >>>>>> + } >>>>>> + >>>>>> + bytestream2_skip(&gb, 3); // flags >>>>>> >>>>>> switch (track->video.projection.type) { >>>>>> case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR: >>>>>> - projection = AV_SPHERICAL_EQUIRECTANGULAR; >>>>>> + if (track->video.projection.private.size == 0) >>>>>> + projection = AV_SPHERICAL_EQUIRECTANGULAR; >>>>>> + else if (track->video.projection.private.size == 20) { >>>>>> + t = bytestream2_get_be32(&gb); >>>>>> + b = bytestream2_get_be32(&gb); >>>>>> + l = bytestream2_get_be32(&gb); >>>>>> + r = bytestream2_get_be32(&gb); >>>>>> + >>>>>> + if (b >= UINT_MAX - t || r >= UINT_MAX - l) { >>>>>> + av_log(NULL, AV_LOG_ERROR, >>>>>> + "Invalid bounding rectangle coordinates " >>>>>> + "%zu,%zu,%zu,%zu\n", l, t, r, b); >>>>> >>>>> Use SIZE_SPECIFIER instead of zu. Msvc doesn't like the latter. >>>>> Same with the mov implementation. >>> >>> Umh? Isn't %zu supported on any msvc version that matters (2015 onwards)? >>> https://connect.microsoft.com/VisualStudio/feedback/details/2083820/printf-format-specifier-zu-is-supported-but-not-documented >> >> We also support 2012 and 2013. SIZE_SPECIFIER becomes zu for gcc and >> such, but Iu for MSVC. > > old software.............. > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel