| Message ID | 20201028225643.30485-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avformat/apngdec: Check for incomplete reads in append_extradata() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
On Wed, 28 Oct 2020, Michael Niedermayer wrote: > Fixes: out array read > Fixes: 26610/clusterfuzz-testcase-minimized-ffmpeg_dem_CONCAT_fuzzer-5631838049271808 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/concatdec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c > index 4b56b61404..0734bc44e1 100644 > --- a/libavformat/concatdec.c > +++ b/libavformat/concatdec.c > @@ -123,7 +123,8 @@ static int add_file(AVFormatContext *avf, char *filename, ConcatFile **rfile, > > proto = avio_find_protocol_name(filename); > proto_len = proto ? strlen(proto) : 0; > - if (proto && !memcmp(filename, proto, proto_len) && > + if (proto && strlen(filename) >= proto_len && > + !memcmp(filename, proto, proto_len) && Why not strncmp? Thanks, Marton > (filename[proto_len] == ':' || filename[proto_len] == ',')) { > url = filename; > filename = NULL; > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer (12020-10-28): > Fixes: out array read > Fixes: 26610/clusterfuzz-testcase-minimized-ffmpeg_dem_CONCAT_fuzzer-5631838049271808 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/concatdec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) This is not the first time such a fix is proposed. If strcmp() does not behave intelligently, then we need an extra function for the task, but duplicating the test all over the place is not a good idea. Regards,
Nicolas George: > Michael Niedermayer (12020-10-28): >> Fixes: out array read >> Fixes: 26610/clusterfuzz-testcase-minimized-ffmpeg_dem_CONCAT_fuzzer-5631838049271808 >> >> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> libavformat/concatdec.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > This is not the first time such a fix is proposed. If strcmp() does not > behave intelligently, then we need an extra function for the task, but > duplicating the test all over the place is not a good idea. > We already have av_strstart() which would also allow us to avoid computing strlen(proto) at all. - Andreas
On Thu, Oct 29, 2020 at 04:22:23PM +0100, Andreas Rheinhardt wrote: > Nicolas George: > > Michael Niedermayer (12020-10-28): > >> Fixes: out array read > >> Fixes: 26610/clusterfuzz-testcase-minimized-ffmpeg_dem_CONCAT_fuzzer-5631838049271808 > >> > >> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >> --- > >> libavformat/concatdec.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > > > > This is not the first time such a fix is proposed. If strcmp() does not > > behave intelligently, then we need an extra function for the task, but > > duplicating the test all over the place is not a good idea. > > > We already have av_strstart() which would also allow us to avoid > computing strlen(proto) at all. patch with av_strstart() posted, av_strstart() was available since a long time so it should have no negative impact on backporting thx [...]
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index 4b56b61404..0734bc44e1 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -123,7 +123,8 @@ static int add_file(AVFormatContext *avf, char *filename, ConcatFile **rfile, proto = avio_find_protocol_name(filename); proto_len = proto ? strlen(proto) : 0; - if (proto && !memcmp(filename, proto, proto_len) && + if (proto && strlen(filename) >= proto_len && + !memcmp(filename, proto, proto_len) && (filename[proto_len] == ':' || filename[proto_len] == ',')) { url = filename; filename = NULL;
Fixes: out array read Fixes: 26610/clusterfuzz-testcase-minimized-ffmpeg_dem_CONCAT_fuzzer-5631838049271808 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/concatdec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)