This is the mail archive of the
insight@sourceware.org
mailing list for the Insight project.
RE: Possible security flaw in Insight
- From: "Dave Korn" <dave dot korn at artimi dot com>
- To: "'Ben Hutchings'" <ben at decadentplace dot org dot uk>,<insight at sources dot redhat dot com>
- Date: Tue, 1 Nov 2005 14:53:35 -0000
- Subject: RE: Possible security flaw in Insight
Ben Hutchings wrote:
> This security advisory explains a bug in some versions of Tcl, which
> may affect Insight.
>
> Ben.
>
> readdir_r considered harmful
> ============================
Well, readdir_r is used in tcl/unix/tclUnixThrd.c as follows:
--------------------------------snip--------------------------------
typedef struct ThreadSpecificData {
char nabuf[16];
struct tm gtbuf;
struct tm ltbuf;
struct {
Tcl_DirEntry ent;
char name[PATH_MAX+1];
} rdbuf;
} ThreadSpecificData;
Tcl_DirEntry *
TclpReaddir(DIR * dir)
{
Tcl_DirEntry *ent;
#ifdef TCL_THREADS
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
#ifdef HAVE_READDIR_R
ent = &tsdPtr->rdbuf.ent;
if (TclOSreaddir_r(dir, ent, &ent) != 0) {
ent = NULL;
}
--------------------------------snip--------------------------------
[ In tclUnixPort.h we have
#ifdef HAVE_STRUCT_DIRENT64
typedef struct dirent64 Tcl_DirEntry;
# define TclOSreaddir readdir64
# define TclOSreaddir_r readdir64_r
#else
typedef struct dirent Tcl_DirEntry;
# define TclOSreaddir readdir
# define TclOSreaddir_r readdir_r
#endif
]
--------------------------------snip--------------------------------
Since PATH_MAX+1 is more than long enough for the longest path possible, and
we only need enough space for a single pathname component, which is at most
NAME_MAX+1[*], I don't see any way this could overflow. In fact, we could
safely reduce the size of that buffer and save some memory, assuming that it's
not used for other purposes as well (which I have _not_ verified).
> Recommendations
> ---------------
> Suggested code for calculating the required buffer size for readdir_r
> follows.
I'm with Zaraza (sp?) on this one. What's wrong with statically sizing it
to NAME_MAX+1, in accordance with the demands of the posix spec?
http://opengroup.org/onlinepubs/009695399/functions/readdir_r.html
" The thread-safe version of the directory reading function returns values in
a user-supplied buffer instead of possibly using a static data area that may
be overwritten by each call. Either the {NAME_MAX} compile-time constant or
the corresponding pathconf() option can be used to determine the maximum sizes
of returned pathnames. "
cheers,
DaveK
[*] http://opengroup.org/onlinepubs/009695399/basedefs/limits.h.html:
PATH_MAX must include +1 for the terminating NUL already. NAME_MAX does not.
--
Can't think of a witty .sigline today....