[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: buffer write bug in mg
- To: tech_(_at_)_openbsd_(_dot_)_org
- Subject: Re: buffer write bug in mg
- From: kjell_(_at_)_pintday_(_dot_)_org
- Date: Tue, 04 Apr 2006 11:56:07 -0600
> Kjell Wooding wrote:
> > While I'm here, use basename() instead of some custom stuff
>
> Building on Linux resulted in this warning:
>
> buffer.c:626: warning: passing arg 1 of `__xpg_basename' discards qualifiers from pointer target type
Ah right, we've hit this before.
> >From basename(3):
>
> CAVEATS
> basename() returns a pointer to internal static storage space that will
> be overwritten by subsequent calls.
That's the openbsd caveat. It's okay here, as we do an immediate copy.
The one you probably mean is the one that says "Both dirname and
basename may modify the contents of path, so copies should be passed
to these functions." which is specific to the linux POSIX basename
(which you get when you include libgen)
If you don't include libgen, you should get the glib version, which
doesn't have this little evil behavior.
> So I'd like to suggest this extra patch:
Bleah. That would mean extra strdups wherever baseame and dirname are
used. This isn't needed under OpenBSD, and you should be able to avoid
it by avoiding the aforementioned include.
Does POSIX *actually* insist that basename and dirname MUST have
evil side effects?
>
> --- /usr/src/usr.bin/mg/buffer.c Tue Apr 4 11:58:14 2006
> +++ buffer.c Tue Apr 4 11:52:40 2006
> @@ -622,8 +622,11 @@ baugname(char *bn, const char *fn, size_
> {
> int count;
> size_t remain, len;
> + char *fn_dup;
>
> - len = strlcpy(bn, basename(fn), bs);
> + fn_dup = strdup(fn);
> + len = strlcpy(bn, basename(fn_dup), bs);
> + free(fn_dup);
> if (len >= bs)
> return (FALSE);
>
And, you missed an error check on that strdup!
-kj
Visit your host, monkey.org