NetBSD Problem Report #40693
From www@NetBSD.org Thu Feb 19 16:28:00 2009
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
by narn.NetBSD.org (Postfix) with ESMTP id 7D18163C0D5
for <gnats-bugs@gnats.netbsd.org>; Thu, 19 Feb 2009 16:28:00 +0000 (UTC)
Message-Id: <20090219162800.43CB063C0CF@narn.NetBSD.org>
Date: Thu, 19 Feb 2009 16:28:00 +0000 (UTC)
From: persgray@gmail.com
Reply-To: persgray@gmail.com
To: gnats-bugs@NetBSD.org
Subject: _gettemp() flawed
X-Send-Pr-Version: www-1.0
>Number: 40693
>Category: lib
>Synopsis: _gettemp() flawed
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: lib-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Feb 19 16:30:00 +0000 2009
>Last-Modified: Fri Feb 20 09:05:03 +0000 2009
>Originator: Vadim Zhukov
>Release: CURRENT
>Organization:
>Environment:
>Description:
After fixing out-of-bounds access in OpenBSD's version of this function, I looked at NetBSD's one. As far as I can see, current implementation of _gettemp() in libc (core function for mk*temp(3)) is flawed by many ways:
- It produces highly predictable (i.e. insecure) values;
- It may (should) cause SIGSEGV when path (template) provided has zero length;
- Maybe more.
Thank you for your attention.
>How-To-Repeat:
/*
* May die if memory exhausted before actual memory allocation
* occurs at page start
*/
#include <stdio.h>
void
main() {
char *s;
size_t sz;
for (sz = 1024;; sz *= 2) {
if ((s = malloc(sz)) == NULL)
err(1, "malloc");
*s = '\0';
mktemp(s);
}
}
>Fix:
I recommend to replace it via OpenBSD's _gettemp() implementation:
http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/stdio/mktemp.c?rev=1.25
>Audit-Trail:
From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: lib/40693: _gettemp() flawed
Date: Thu, 19 Feb 2009 21:07:07 +0200
On Thu, 19 Feb 2009, persgray@gmail.com wrote:
> After fixing out-of-bounds access in OpenBSD's version of this
> function, I looked at NetBSD's one. As far as I can see, current
> implementation of _gettemp() in libc (core function for mk*temp(3)) is
> flawed by many ways:
>
> - It produces highly predictable (i.e. insecure) values;
> - It may (should) cause SIGSEGV when path (template) provided has zero
> length;
> - Maybe more.
I am not sure that the "highly predictable values" is a real problem,
except for callers who use the deprecated mktemp(3) interface. Users
of the mkstemp(3) and mkdtemp(3) inetrfaces should get the advertised
uniqueness guarantees.
I do see a problem when strlen(path) = 0 (it tries to access path[-1]).
> I recommend to replace it via OpenBSD's _gettemp() implementation:
>
> http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/stdio/mktemp.c?rev=1.25
That implementation calls strlen(path) without verifying that path != NULL.
--apb (Alan Barrett)
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: lib/40693: _gettemp() flawed
Date: Thu, 19 Feb 2009 20:19:36 +0100
How is crashing (or not) a problem when passing invalid arguments to a
libary function?
Martin
From: Vadim Zhukov <persgray@gmail.com>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: lib/40693: _gettemp() flawed
Date: Fri, 20 Feb 2009 12:00:02 +0300
On 19 February 2009 =C7. 22:15:03 Alan Barrett wrote:
> The following reply was made to PR lib/40693; it has been noted by
> GNATS.
>
> From: Alan Barrett <apb@cequrux.com>
> To: gnats-bugs@NetBSD.org
> Cc:
> Subject: Re: lib/40693: _gettemp() flawed
> Date: Thu, 19 Feb 2009 21:07:07 +0200
>
> On Thu, 19 Feb 2009, persgray@gmail.com wrote:
> > After fixing out-of-bounds access in OpenBSD's version of this
> > function, I looked at NetBSD's one. As far as I can see, current
> > implementation of _gettemp() in libc (core function for mk*temp(3))
> > is flawed by many ways:
> >
> > - It produces highly predictable (i.e. insecure) values;
> > - It may (should) cause SIGSEGV when path (template) provided has
> > zero length;
> > - Maybe more.
>
> I am not sure that the "highly predictable values" is a real problem,
> except for callers who use the deprecated mktemp(3) interface. Users
> of the mkstemp(3) and mkdtemp(3) inetrfaces should get the advertised
> uniqueness guarantees.
Yes, mkstemp(3) and mkdtemp(3) are safe enough, but there is some legacy=20
software that was not ported to using those routines. Of course, it's=20
your (by "you" I mean NetBSD developers) politics whether to care or not=20
about such programs. But if you don't care, you can just remove=20
mktemp(3). And even mktemp(3) may be used safely using "open(O_CREAT|
O_EXCL); fdopen()" dance.
> I do see a problem when strlen(path) =3D 0 (it tries to access
> path[-1]).
>
> > I recommend to replace it via OpenBSD's _gettemp() implementation:
> >
> > http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/stdio/mktemp.c?r
> >ev=3D1.25
>
> That implementation calls strlen(path) without verifying that path !=3D
> NULL.
The only function for which POSIX says it accepts NULL pointer is=20
tmpnam() - it is logical to assume (though assuming anything in standard=20
is bad practice in general, of course) that other functions should not=20
care about NULL pointer. Moreover, open(2) returns EFAULT for NULL=20
pointer passed, not EINVAL - this makes a little confusion.
If you care, it's not more than adding
_DIAGASSERT(path !=3D NULL);
to the OpenBSD's implementation. I can make a full patch, if you wish.=20
And in NetBSD it's already checked in __gettemp() callers, and if=20
someone will call __gettemp() directly they'll by definition shoot in=20
their foot.
=2D-=20
Best wishes,
Vadim Zhukov
From: Vadim Zhukov <persgray@gmail.com>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: lib/40693: _gettemp() flawed
Date: Fri, 20 Feb 2009 12:02:08 +0300
On 19 February 2009 =C7. 22:20:04 Martin Husemann wrote:
> The following reply was made to PR lib/40693; it has been noted by
> GNATS.
>
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@NetBSD.org
> Cc:
> Subject: Re: lib/40693: _gettemp() flawed
> Date: Thu, 19 Feb 2009 20:19:36 +0100
>
> How is crashing (or not) a problem when passing invalid arguments to
> a libary function?
It's all about what to consider "invalid". Of course, this is NetBSD=20
developer's decision, not my.
Thanks to all for you attention.
=2D-=20
Best wishes,
Vadim Zhukov
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.