On Tue, Feb 03, 2015 at 09:44:13PM +0100, Reimar Döffinger wrote: > On Tue, Feb 03, 2015 at 07:04:11PM +0100, wm4 wrote: > > This could overflow and crash at least on 32 bit systems. > > --- > > libavformat/mpc8.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/mpc8.c b/libavformat/mpc8.c > > index a15dc25..d6ca338 100644 > > --- a/libavformat/mpc8.c > > +++ b/libavformat/mpc8.c > > @@ -91,7 +91,7 @@ static int mpc8_probe(AVProbeData *p) > > size = bs_get_v(&bs); > > if (size < 2) > > return 0; > > - if (bs + size - 2 >= bs_end) > > + if (size >= bs_end - bs + 2) > > return AVPROBE_SCORE_EXTENSION - 1; // seems to be valid MPC > > but no header yet > > Seems ok to me, but for consistency/ease of checking
applied
> I'd suggest changing while (bs < bs_end + 3) to this style
> as well.
> However there is one more issue:
> bs_get_v can overflow.
> And as the compiler can assume that signed overflow
> will not happen, that case is not certain to be
> caught by the size < 2 check, and thus these cases
> might escape all checks.
> Stupid question: Why do we support size values of whole
> 64 bits?
> Nobody has invented any storage media where one could store that much.
> Changing the limit in bs_get_v from 10 to e.g. 6 or 7 should
> avoid the issue...
i would suggest:
@@ -57,7 +57,7 @@ typedef struct {
static inline int64_t bs_get_v(const uint8_t **bs)
{
- int64_t v = 0;
+ uint64_t v = 0;
int br = 0;
int c;
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
