From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <insight-return-5508-listarch-insight=sources.redhat.com@sourceware.org>
Received: (qmail 7233 invoked by alias); 1 Nov 2005 14:53:40 -0000
Mailing-List: contact insight-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Subscribe: <mailto:insight-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/insight/>
List-Post: <mailto:insight@sourceware.org>
List-Help: <mailto:insight-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: insight-owner@sourceware.org
Received: (qmail 7225 invoked by uid 22791); 1 Nov 2005 14:53:37 -0000
Received: from host217-40-213-68.in-addr.btopenworld.com (HELO SERRANO.CAM.ARTIMI.COM) (217.40.213.68)
    by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Tue, 01 Nov 2005 14:53:37 +0000
Received: from espanola ([192.168.1.110]) by SERRANO.CAM.ARTIMI.COM with Microsoft SMTPSVC(6.0.3790.1830);
	 Tue, 1 Nov 2005 14:53:34 +0000
From: "Dave Korn" <dave.korn@artimi.com>
To: "'Ben Hutchings'" <ben@decadentplace.org.uk>,
	<insight@sources.redhat.com>
Subject: RE: Possible security flaw in Insight
Date: Tue, 01 Nov 2005 14:53:00 -0000
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 7bit
In-Reply-To: <20051101035927.GJ27885@decadentplace.org.uk>
Message-ID: <SERRANO1E9GQvnbEG1000000071@SERRANO.CAM.ARTIMI.COM>
X-SW-Source: 2005-q4/txt/msg00005.txt.bz2

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....