[FFmpeg-devel] [PATCH 2/4] Implement ocv_dilate libopencv filter wrapper.

[Stefano Sabatini]

On date Monday 2010-11-15 15:34:28 +0100, Stefano Sabatini encoded:
> On date Friday 2010-11-12 23:10:08 +0100, Michael Niedermayer encoded:
[...]
> > > +    buf = av_malloc(size + 1);
> > > +    if (!buf) {
> > > +        fclose(file);
> > > +        return AVERROR(ENOMEM);
> > > +    }
> > > +    fread(buf, 1, size, file);
> > 
> > this looks exploitable or will at least crash
> 
> I don't know what you're talking about. Note that this is the same
> code as cmdutils.c:read_file.
> 
> > > +    buf[size++] = 0;
> > > +    fclose(file);
> > > +
> > > +    /* prescan file to get the number of lines and the maximum width */
> > > +    w = 0;
> > > +    for (i = 0; i < size; i++) {
> > > +        if (buf[i] == '\n') {
> > > +            if (++(*rows) <= 0) {
> > > +                av_log(log_ctx, AV_LOG_ERROR, "Overflow on the number of rows in the file\n");
> > > +                return AVERROR(EINVAL);
> > > +            }
> > > +            *cols = FFMAX(*cols, w);
> > > +            w = 0;
> > > +        } else if (++w <= 0) {
> > > +            av_log(log_ctx, AV_LOG_ERROR, "Overflow on the number of columns in the file\n");
> > > +            return AVERROR(EINVAL);
> > > +        }
> > 
> > these tests are useless signed overflow is undefined, any good compiler should
> > remove these tests and you still get the overflow and whatever happens without
> > them triggering
> 
> Is:
>             if (*rows == INT_MAX) {
>                 av_log(log_ctx, AV_LOG_ERROR, "Overflow on the number of rows in the file\n");
>                 return AVERROR(EINVAL);
>             }
>             ++(*rows);
> OK?
> 
> [...]
> > > +    if (*rows > sizeof(int) * INT_MAX / *cols) {
> > 
> > 4*INT_MAX
> > is this a joke?
> 
> changed to something saner:
>     if (*rows > (INT_MAX / (sizeof(int)) / *cols)) {
>         av_log(log_ctx, AV_LOG_ERROR, "File with size %dx%d is too big\n",
>                *rows, *cols);
>         return AVERROR_INVALIDDATA;
>     }
>     if (!(*values = av_mallocz(sizeof(int) * *rows * *cols)))
>         ...
[...]

Ping.
-- 
FFmpeg = Faithless Frightening MultiPurpose Encoding/decoding Geisha