Redundant check in validation of an extent

[wantehchang]

In src/read.c, when avifDecoderDataCalcItemPtr() validates an extent, it performs two checks. Here I use the single-extent case as an example:

        avifDecoderItemExtent * extent = &item->extents.extent[0];
        if (extent->offset > offsetBuffer->size) {
            return NULL; 
        }   
        uint64_t offsetSize = (uint64_t)extent->offset + (uint64_t)extent->size;
        if (offsetSize > (uint64_t)offsetBuffer->size) {
            return NULL; 
        }

Since extent->size is of an unsigned type (uint32_t), if the first condition is true, then the second condition is also true. Therefore the first check is redundant.

We can deal with this in two ways.

1. Remove the redundant first check. This will be equivalent to the current code.

2. Make the first check stricter:

        if (extent->offset >= offsetBuffer->size) {
            return NULL; 
        }

This will effectively disallow a zero-length extent (and a zero-length offsetBuffer).

Joe, which do you prefer?

[wantehchang]

Hmm... Section 8.11.3.3 of ISO BMFF says that an extent_length value of 0 does not mean the length of the extent is 0, but rather the length of the extent is the length of the entire referenced container. So we will need to handle that case correctly.