[FFmpeg-devel] av_strlcpy() size parameter signedness
Sat Sep 29 11:05:13 CEST 2007
On Fri, Sep 28, 2007 at 09:27:02PM -0400, Rich Felker wrote:
> On Sat, Sep 29, 2007 at 01:37:48AM +0200, Michael Niedermayer wrote:
> > Hi
> > currently the buffer size parameter for av_strlcpy() (and friends)
> > is unsigned this makes sense and is logic but it has a flaw
> > that is if a negative value is mistakely used something very bad happens
> > how can a negative value be assigned?
> > av_strlcpy(... FFMIN(buf_size, something))
> > with something being <0 and buf_size signed
> This sounds like a bogus construct.
what i had in mind was:
av_strlcpy(buf, p, FFMIN(buf_size, strchr(p, ':') - p + 1));
code like that was in the recently approved IPv6 patch
and yes i agree buf_size should be changed to unsigend ...
> We should find out why values that
> can be negative are being used in sizes like this. A hack to make the
> problem go away is not appropriate; it's just covering up the
> underlying bug.
sure should the underlaying bug be fixed but
the underlaying bug is likely harmless compared to what av_strlcpy will do
due to it, and we can only fix bugs which have been found ...
also your argument could be used against the existence of strlcpy itself
why not consider it a bug if a string is longer than the buffer?
what the function does is trying to prevent buffer overflows at the
expense of possibly hiding bugs ...
really i do prefer a <0 size bug to be hidden instead of having a
buffer ovreflow, also we could just call abort() ar just dereference NULL
instead of using 0 that would not cover the bug up but still prevent the
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel