[vlc-devel] [PATCH] input/control: fix off-by-one bounds check

Filip Roséen filip at atch.se
Wed Mar 1 20:11:43 CET 2017


Hi j-b,

On 2017-03-01 17:46, Jean-Baptiste Kempf wrote:

> > -            if( !priv->i_title || priv->i_title < *pi_title_to_fetch )
> > +            if( priv->i_title <= *pi_title_to_fetch )
> 
> Why remove the first part of the test?

Because it does not add any significance to the *if-clause*; as we are
dealing with indices `priv->i_title` having a value of `0` will always
be less than or equal to `*pi_title_to_fetch`, meaning that we should
return `VLC_EGENERIC`.

I am not sure what the original reason was for having
`!priv->i_title`, perhaps:

  - it was a mental shortcut stating that an empty set shall always
    result in `VLC_EGENERIC` - but that is now solved by using `<=`
    instead of `<`, or;

  - it was simply the `if` at `src/input/control.c:348` that was used
    as a template, and then negated in a redundant fashion.

        if( priv->i_title && priv->i_title > *pi_req_title_offset )

------------------------------------------------------------------------

However...
------------------------------------------------------------------------

Though, there is a possibility for `*pi_title_to_fetch` to be negative
because of invalid values set for `"title"`-variable. Even though that
should not happen, it is perhaps best to make sure that it simply
cannot happen.

See attached patch.

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170301/c9d54e6b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-input-control-INPUT_GET_SEEKPOINTS-prevent-out-of-bo.patch
Type: text/x-diff
Size: 1371 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170301/c9d54e6b/attachment.patch>


More information about the vlc-devel mailing list