ons 2019-11-27 klockan 20:00 +0100 skrev Paul B Mahol: > On 11/27/19, James Almer <[email protected]> wrote: > > On 11/27/2019 3:42 PM, Paul B Mahol wrote: > > > On 11/27/19, Tomas Härdin <[email protected]> wrote: > > > > tis 2019-11-26 klockan 20:29 +0100 skrev Paul B Mahol: > > > > > On 11/26/19, James Almer <[email protected]> wrote: > > > > > > On 11/26/2019 6:47 AM, Paul B Mahol wrote: > > > > > > > On 11/25/19, Tomas Härdin <[email protected]> wrote: > > > > > > > > mån 2019-11-25 klockan 22:09 +0100 skrev Paul B Mahol: > > > > > > > > > Signed-off-by: Paul B Mahol <[email protected]> > > > > > > > > > +static int decode_mvdv(MidiVidContext *s, AVCodecContext > > > > > > > > > *avctx, > > > > > > > > > AVFrame > > > > > > > > > *frame) > > > > > > > > > +{ > > > > > > > > > + GetByteContext *gb = &s->gb; > > > > > > > > > + GetBitContext mask; > > > > > > > > > + GetByteContext idx9; > > > > > > > > > + uint16_t nb_vectors, intra_flag; > > > > > > > > > + const uint8_t *vec; > > > > > > > > > + const uint8_t *mask_start; > > > > > > > > > + uint8_t *skip; > > > > > > > > > + int mask_size; > > > > > > > > > + int idx9bits = 0; > > > > > > > > > + int idx9val = 0; > > > > > > > > > + int num_blocks; > > > > > > > > > + > > > > > > > > > + nb_vectors = bytestream2_get_le16(gb); > > > > > > > > > + intra_flag = bytestream2_get_le16(gb); > > > > > > > > > + if (intra_flag) { > > > > > > > > > + num_blocks = (avctx->width / 2) * (avctx->height / > > > > > > > > > 2); > > > > > > > > > > > > > > > > Will UB if width*height/4 > INT_MAX > > > > > > > > > > > > > > > > > + } else { > > > > > > > > > + int skip_linesize; > > > > > > > > > + > > > > > > > > > + num_blocks = bytestream2_get_le32(gb); > > > > > > > > > > > > > > > > Might want to use uint32_t so this doesn't lead to weirdness on > > > > > > > > 32-bit > > > > > > > > > > > > > > > > > + skip_linesize = avctx->width >> 1; > > > > > > > > > + mask_start = gb->buffer_start + bytestream2_tell(gb); > > > > > > > > > + mask_size = (avctx->width >> 5) * (avctx->height >> > > > > > > > > > 2); > > > > > > > > > > > > > > > > This can also UB > > > > > > > > > > > > > > > > /Tomas > > > > > > > > > > > > > > > > > > > > > > Nothing of this can actually happen. > > > > > > > > This assumes max_pixels will never be increased above INT_MAX. "64k" > > > > video is most definitely within the range of possibility in the coming > > > > years, if it isn't already. Film archival and DPX come to mind. > > > > > > > > > > It can and i'm fairly sure it will happen as soon as the fuzzer > > > > > > starts > > > > > > testing this decoder using big dimensions. > > > > > > > > > > I'm not that guy doing such work. Please stop bikesheding those > > > > > patches for once. > > > > > > > > This reads like an admission of pushing insecure code via "not my > > > > problem" > > > > > > > > > > You don't need asserts here, you just need to check the calculations > > > > > > will not overflow. Do something like "if ((int64_t)avctx->width * > > > > > > avctx->height / 4 > INT_MAX) return AVERROR_INVALIDDATA" and call > > > > > > it a > > > > > > day. > > > > > > Also, maybe num_blocks should be unsigned, seeing you set it using > > > > > > bytestream2_get_le32() for P-frames. > > > > > > > > > > No decoder does this. > > > > > > > > zmbv does > > > > > > > > All this is really about the lack of any way to reason about > > > > assumptions like "dimensions can't be larger than X*Y" at compile time, > > > > which is a thing I've been pointing out on this list for a while now. > > > > > > > > > > Nobody tells you not to fix it yourself. > > > > Just add a the suggested overflow checks, Christ. It's a one line > > addition each. What do you gain arguing like this? > > Than next day he or you will come with another great idea.
Great ideas like pushing inevitable 0days? /Tomas _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
