RE: Possible security flaw in Insight

["Dave Korn" <dave dot korn at artimi dot com>]

Ben Hutchings wrote:
> [I'm not subscribed to the mailing list, so please continue to cc
> replies to me.]
> 
> Dave Korn wrote:
>> 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;
> 
> In some versions of Tcl (8.4.2 to 8.5a2 inclusive), the dimension of the
> name field is MAXNAMLEN+1, not PATH_MAX+1.

  The code I quoted was from /src, which hasn't had a new version merged in
from upstream recently and is still on 8.4.1.  In the main sourceforge
repository, the whole problem was rendered moot by the fix for

http://sourceforge.net/tracker/index.php?func=detail&aid=1095909&group_id=1089
4&atid=110894

which has been applied to both HEAD and 8.4 branch, and therefore will be
picked up any time the sourceware tcl ever does get up-merged.

  So, I don't believe this is a problem for insight: in the current code, the
buffer is big enough, and in any future version of the code, readdir_r is no
longer used.
 
> <snip>
>>   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? <snip> 
> 
> NAME_MAX isn't required to be defined (and MAXNAMLEN isn't even
> mentioned by POSIX, though it is equivalent on many systems).  

  Oh well, autoconf can help with that, as could falling back to PATH_MAX if
it exists.  In practice, it doesn't seem to really be an issue: the use of
PATH_MAX in that structure is unconditional and in fact not autoconfiscated,
and nobody has reported that they've had trouble building for lack of it.

> GNU/Hurd
> doesn't define it, for example, because there is no practical limit on
> name lengths there.

  Umm, then surely GNU/Hurd is insane and cannot be used because there is no
way to ever know how much space to allocate for a dirent?


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....