On 21.07.2019, at 00:36, Lynne <d...@lynne.ee> wrote: > Jul 20, 2019, 11:08 PM by mich...@niedermayer.cc: > >> Fixes: Timeout (22 -> 7 sec) >> Fixes: >> 15173/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HQX_fuzzer-5662556846292992 >> >> Found-by: continuous fuzzing process >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >> --- >> libavcodec/hqx.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/libavcodec/hqx.c b/libavcodec/hqx.c >> index bc24ba91d1..8639d77a41 100644 >> --- a/libavcodec/hqx.c >> +++ b/libavcodec/hqx.c >> @@ -471,6 +471,10 @@ static int hqx_decode_frame(AVCodecContext *avctx, void >> *data, >> avctx->height = ctx->height; >> avctx->bits_per_raw_sample = 10; >> >> + if (avctx->coded_width / 16 * (avctx->coded_height / 16) * >> + (100 - avctx->discard_damaged_percentage) / 100 > 8LL * avpkt->size) >> + return AVERROR_INVALIDDATA; >> + >> > > Not only are you ignoring my and others opinion, not only you still continue > sending these awful patches, > you've just submitted by far the worst one I've ever seen thinking its okay. > Patches like these motivate developers to not even bother including test > samples for new decoders, or even write them. Myself included. Doing exactly > the opposite of what this system's meant to help. > Sure, you sent this for review, but how can you even consider this utterly > ridiculous hack for a problem that doesn't exist even worthy for review in > the first place? Just what the fuck?
I kind of understand your point of view, and the fuzzer complaining should not be an excuse to skip writing a commit message with some motivation for example, but I think you are a bit over the top. There is already a discard_damaged_percentage and there is a point that maybe if a packet is obviously too broken to discard it with minimal overhead. Those do not seem like utterly ridiculous ideas as your reply makes them out to me. Nor is the idea of having some hardening against all too easy DoS attacks in some use-cases. Also some value in just having the fuzzer run efficiently (it also discovers quite some real issues). I agree it's hackish, I don't know the code enough to know it's correct, it needs to be properly documented (Michael, please, if you add such non-obvious, and especially heuristic checks, there needs to be some comment telling people the idea behind it and how to easily verify its correctness etc.). With that in mind, can you maybe see why I do think that discussing such patch proposals does have merit? Can we maybe come up with some compromise without being mad at each other? Maybe some of these likely to be more controversial could be submitted as RFC instead of patch first? Best regards, Reimar _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
