Patch for bin/54446
Garrett Cooper
yanefbsd at gmail.com
Wed Nov 26 04:44:41 PST 2008
On Nov 26, 2008, at 4:20 AM, insomniac wrote:
> On Tue, 25 Nov 2008 19:33:37 -0800
> Xin LI <delphij at delphij.net> wrote:
>
>> I have made a small change: use malloc() here and use strlcpy().
>> Other parts looks just fine.
>>
>> (BTW I think we need to cc portmgr@ for approval)
>
> I merged your fix with Garrett Cooper's one (I forgot to free()
> before
> a return) and also mailed portmgr@ for approval. The new patch is
> located at the same link:
>
> http://insomniac.slackware.it/plist.c.diff
>
> Moreover, I will fix style(9) in a second step.
>
> Best regards,
> --
> Andrea Barberio
Xin Li brought up a really good point (that I remembered when I first
looked at the code, but forgot to mention later):
calloc is nothing more than a malloc + bzero on many OSes (FreeBSD has
moved away from this methodology), so using calloc sparingly is for
the best. It really doesn't buy you much here anyhow since you're
overwriting everything with a copied string anyhow...
As for the potential security issue I mentioned earlier, it could be
definitely be done if someone has the knowledge and was running with
similar privs or had write access to the parent / child(ren) symlink.
One of the ways of solving this issue may be to use flock(2) *shrugs*,
as it would block other callers from modifying the file or the symlink
(our concern would be symlinks I would think).
This kind of race condition situation is part of the driving force
between using fstat vs lstat vs stat.
HTH,
-Garrett
More information about the freebsd-current
mailing list