Date: Mon, 13 Aug 2001 03:42:19 +0900 From: "Akinori MUSHA" <knu@iDaemons.org> To: Mike Barcroft <mike@FreeBSD.org> Cc: audit@FreeBSD.org Subject: Re: adding -P option to pkg_delete(1) Message-ID: <86snex15o4.wl@archon.local.idaemons.org> In-Reply-To: <20010812140750.B29132@coffee.q9media.com> References: <86elqphctp.wl@archon.local.idaemons.org> <86d761hijs.wl@archon.local.idaemons.org> <20010812140750.B29132@coffee.q9media.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Thanks for the review,
At Sun, 12 Aug 2001 14:07:50 -0400,
Mike Barcroft wrote:
> > /*
> > + * Check if the given filename looks like a shared library.
> > + */
> > +static Boolean
> > +is_shlib(char *filename)
> > +{
> > + char *p, *q;
> > +
> > + /* basename */
> > + if (NULL != (p = strrchr(filename, '/')))
> > + p++;
> > + else
> > + p = filename;
> > +
> > + /* empty filename or dotfile? */
> > + if (*p == '\0' || *p == '.')
> > + return FALSE;
> > +
> > + /* do "strrstr" for .so */
> > + if (NULL == (q = strstr(p + 1, ".so")))
> > + return FALSE;
> > + while (NULL != (p = strstr(q += 3, ".so")))
> > + q = p;
> > +
> > + /* skip version numbers */
> > + while (*q) {
> > + if (*q != '.' || !isdigit(*++q))
> > + return FALSE;
> > + while (isdigit(*++q))
> > + ;
> > + }
> > +
> > + return TRUE;
> > +}
> [...]
>
> This could probably be written better, so that you don't have to walk
> filename so many times.
Yes, alternatively you could write it as follows, for example:
/* [^\/]\.so(\.\d+)*$ */
static Boolean
is_shlib(char *filename)
{
char *p;
p = strrchr(filename, 's');
if (p == NULL || p[1] != 'o' ||
p - filename < 2 || p[-1] != '.' || p[-2] == '/')
return FALSE;
p += 2;
/* skip version numbers */
while (*p) {
if (*p != '.' || !isdigit(*++p))
return FALSE;
while (isdigit(*++p))
;
}
return TRUE;
}
(But I don't like this ;)
> [...]
> > - sprintf(tmp, "%s/%s", Where, p->name);
> > + sprintf(tmp, "%s/%s", Where, last_file);
> [...]
>
> I don't see any checks to ensure that this won't overflow tmp, so you
> should probably use snprintf(3) instead.
Indeed, and that kind of code is everywhere in the pkg_install
sources. They'll need overall audit.
> The rest of the patch looks reasonable, but I agree with Sheldon that
> this shouldn't be merged into -STABLE until after 4.4-RELEASE.
I see.
--
/
/__ __ Akinori.org / MUSHA.org
/ ) ) ) ) / FreeBSD.org / Ruby-lang.org
Akinori MUSHA aka / (_ / ( (__( @ iDaemons.org / and.or.jp
"Freeze this moment a little bit longer, make each impression
a little bit stronger.. Experience slips away -- Time stand still"
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86snex15o4.wl>