Servus Michael,
On Mon, Mar 10, 2025 at 9:32 PM Michael Niedermayer
<mich...@niedermayer.cc> wrote:
>
> Hi
>
> On Sun, Mar 09, 2025 at 04:52:25PM +0100, Manuel Lauss wrote:
> > Hi Michael,
> >
> > On Sat, Mar 8, 2025 at 8:11 PM Michael Niedermayer
> > <mich...@niedermayer.cc> wrote:
> > >
> > > Hi Manuel
> > >
> > > On Tue, Mar 04, 2025 at 06:07:18PM +0100, Manuel Lauss wrote:
> > > > The left and top parameters of an FOBJ are signed values.
> > > >
> > > > Signed-off-by: Manuel Lauss <manuel.la...@gmail.com>
> > > > ---
> > > > v4: revert v3, it arose due to a misunderstanding
> > > > v3: change the bytestream accessor to signed too
> > > > v2: no changes
> > > > libavcodec/sanm.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
> > > > index a4f0a28c7c..71dbac4320 100644
> > > > --- a/libavcodec/sanm.c
> > > > +++ b/libavcodec/sanm.c
> > > > @@ -1238,8 +1238,8 @@ static int old_codec48(SANMVideoContext *ctx, int
> > > > width, int height)
> > > > static int process_frame_obj(SANMVideoContext *ctx)
> > > > {
> > > > uint16_t codec = bytestream2_get_le16u(&ctx->gb);
> > > > - uint16_t left = bytestream2_get_le16u(&ctx->gb);
> > > > - uint16_t top = bytestream2_get_le16u(&ctx->gb);
> > > > + int16_t left = bytestream2_get_le16u(&ctx->gb);
> > > > + int16_t top = bytestream2_get_le16u(&ctx->gb);
> > > > uint16_t w = bytestream2_get_le16u(&ctx->gb);
> > > > uint16_t h = bytestream2_get_le16u(&ctx->gb);
> > >
> > > Does the following code also handle all error conditions that
> > > negative left/top could now trigger ?
> >
> > For the LucasArts titles that sanm.c currently supports well,
> > no negative values are ever encountered.
> > I let ffplay run through maybe 1/3 of the Rebel Assault 1 videos,
> > which are the only ones that make use of negative values, but
> > didn't encounter any crashes; mostly because the codecs it
> > uses aren't supported by ffmpeg/sanm (yet).
>
> My concern is not that it crashes my concern is that manually craftet
> files could result in arbitrary code execution if theres any out of
> array accesses.
> Did you check that negative values are safe in that respect ?
I looked over the code and came up with something new.
Please disregard this and the 3/3 patch, I will send a more
comprehensive patchset for sanm shortly,
which hopefully also addresses your concern.
Thanks for the review!
Manuel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
