[Tcl-bugs] [ tcl-Bugs-979640 ] Buffer overruns mixing C and Tcl environment changes

[SourceForge.net <no...@so...>]

Bugs item #979640, was opened at 2004-06-25 10:30
Message generated for change (Comment added) made by eyekode
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=979640&group_id=10894

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: 08. Environment Variables
Group: obsolete: 8.5a1
Status: Closed
Resolution: Fixed
Priority: 9
Private: No
Submitted By: Steve Bold (stevebold)
Assigned to: Jeffrey Hobbs (hobbs)
Summary: Buffer overruns mixing C and Tcl environment changes

Initial Comment:
I have an application which embeds Tcl but which has
other components not using Tcl. The app runs Tcl
scripts which modify the environment but the non-Tcl
components also modify the environment using direct
calls to the C function putenv().

The original problem I encountered was that the
application sometimes crashed on startup when run on
Linux using Tcl 8.4.3 and the crash occurred when in
a call to TclSetEnv(). Debugging the startup sequence
using Purify on Solaris showed Array Buffer Write
(ABW) errors when Tcl modifies the environment,
though on Solaris this did not cause a crash. I have
since reproduced the problem using 8.5a1 and reduced
the application to a small test case involving one
Tcl script and one C file, which are given below:

/* CrashEnv.c - compile as a shared library CrashEnv.so 
*/
#include <tcl.h>
#include <stdlib.h>

static int myCmd(ClientData dummy,Tcl_Interp* 
interp,int objc,Tcl_Obj* CONST* objv)
{
   putenv("a=b");
}


int Envcrash_Init(Tcl_Interp* interp)
{
   if (Tcl_InitStubs(interp,"8.4",0) == NULL) {
      return TCL_ERROR;
   }

   Tcl_CreateObjCommand(interp,"myCmd",myCmd,0,0);
   return TCL_OK;
}
/* End CrashEnv.c */

# CrashEnv.tcl - source in a tclsh built using Purify and 
see an ABW error.
set env(m) 1
load [pwd]/EnvCrash.so
myCmd
set env(l) 2; # ABW here.
# end CrashEnv.tcl


I believe the root of problem is this:

When TclSetEnv() is first asked to add a variable to
the environment, it determines that the length of the
current environment is 10. It needs to allocate a new
environ array sized to at least 12, allowing space
for the new entry and null terminator.

It actually allocates 15 elements. This gives it
space for an extra 3 variables, reducing the
frequence of reallocations.

Subsequently the application makes a direct call to
putenv() and it finds it needs to add a variable to
the environment. It sees that there are 11 variables
in the environment. With a new variable and
terminator the required length is 13, so it allocates
this size as the new environ array.

The script then modifies the environment again,
triggering another call to TclSetEnv(). It sees there
are 12 variables in the environment. With the new
variable and null terminator, a length of 14 is
needed. It remembers that it has previously allocated
space for 15 and so does not extend the array.
Unfortunately, this isn't safe.

It is possible that some implementations of putenv()
employ similar optimisations, so the same problem
might possibly occur the other way around. I conclude
that you cannot allow multiple components to modify
environ.

I can work around the problem by setting $(ENV_FLAGS)
to -DUSE_PUTENV in the Makefile. Possibly this is a
valid fix for the problem?


Purify output from above test using 8.5a1

****  Purify 
instrumented /eeg/home/stevebo/tcl8.5a1/unix/tclsh 
(pid 1476)  ****
ABW: Array bounds write:
  * This is occurring while in:
   TclSetEnv      [tclEnv.c:214]
   EnvTraceProc   [tclEnv.c:561]
   TclCallVarTraces [tclTrace.c:2516]
   TclPtrSetVar   [tclVar.c:1679]
   Tcl_ObjSetVar2 [tclVar.c:1515]
   Tcl_SetObjCmd  [tclVar.c:1286]
   TclEvalObjvInternal [tclBasic.c:3147]
   Tcl_EvalEx     [tclBasic.c:3642]
   Tcl_FSEvalFileEx [tclIOUtil.c:1665]
   Tcl_Main       [tclMain.c:380]
   main           [tclAppInit.c:90]
   _start         [crt1.o]
  * Writing 4 bytes to 0xaa604 in the heap.
  * Address 0xaa604 is 1 byte past end of a malloc'd 
block at 0xaa4d8 of 300 bytes.
  * This block was allocated from:
   malloc         [rtlib.o]
   putenv         [libc.so.1]
   myCmd          [EnvCrash.c]
   TclEvalObjvInternal [tclBasic.c:3147]
   Tcl_EvalEx     [tclBasic.c:3642]
   Tcl_FSEvalFileEx [tclIOUtil.c:1665]
   Tcl_Main       [tclMain.c:380]
   main           [tclAppInit.c:90]
   _start         [crt1.o]


----------------------------------------------------------------------

Comment By: Salem Ganzhorn (eyekode)
Date: 2011-10-26 15:58

Message:
I misread the code the first time. The drawback to using -DUSE_PUTENV is
that "unset env(foo)" leaves "foo" in the environment. It just changes the
value to "" (which is what Steve said in the first place :)).

----------------------------------------------------------------------

Comment By: Salem Ganzhorn (eyekode)
Date: 2011-10-25 20:33

Message:
I have hit this bug in my application as well (Linux platform). We are
using Tcl 8.4.5 and unfortunately I cannot upgrade the version of Tcl for
quite some time. Looks like I have the following options:
1) define USE_PUTENV
2) attempt to backport this change into my 8.4.5 source tree.
3) rewrite TclSetEnv/TclUnsetEnv to use libc's setenv() and unsetenv().

I will try 1 first. I don't yet understand the drawbacks. It looks like it
should work the same. Can anyone explain why Tcl by default directly
modifies environ?

#2 is not straight forward but I may give it a shot.

#3 seems pretty simple. Unfortunately I can't use a simple
-DTclSetEnv=setenv because setenv() takes 3 args. Note unix/Makefile makes
it look like this is all that is needed... not sure when that line would
work.

TIA for any feedback!



----------------------------------------------------------------------

Comment By: Jeffrey Hobbs (hobbs)
Date: 2005-10-05 08:04

Message:
Logged In: YES 
user_id=72656

Applying the attached patch for 8.4.12 and 8.5a2.  Please
test it out and confirm that it fixes your problem.  It
addresses both the USE_PUTENV and the raw environ change issues.

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2005-03-17 07:54

Message:
Logged In: NO 

This problem is also a big issue if Tcl is used with Cygwin,
especially together with expect (e.g. expect-20021218-1 or
expect-20030128-1). Running Cygwin's expect.exe in an env
without a TZ variable every first call of the expect
statement creates this variable - unfortunately with an
invalid value in most cases due to a bug in Cygwin's
localtime.cc. Nevertheless Tcl's memory management didn't
get rid of this env change and sooner or later it crashes
with STATUS_ACCESS_VIOLATION and a stackdump is written to
expect.exe.stackdump file.

----------------------------------------------------------------------

Comment By: Steve Bold (stevebold)
Date: 2004-12-17 12:38

Message:
Logged In: YES 
user_id=810219

Defining USE_PUTENV doesn't work so well on UNIX, since it 
prevents
scripts from unsetting environment variables.

I have implemented a better fix that uses putenv() when 
setting environment
variables and direct environ[] access when modifying them.

This fixes the buffer overun problem and allows unsetting. 
The fix
does not change the Windows behaviour, where putenv() can 
safely
be used for setting and unsetting environment variables.

The fix is submitted as 979640.patch and attached to this 
bug report.
The patch was derived from 8.5a2. With it, all tests on 
Solaris pass.
On Windows I see test failures on expr.test and winDde.test,
but these occur with or without the patch.

Please consider incorporating this into 8.5 or let me
know what further work is needed to achieve a valid fix
for this issue.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=979640&group_id=10894