Buffer overflow in script compiler?

I wasn't able to track this down to an exact code location, but something seems to be wrong with the memory usage of the StringStack used during script execution.

I was wondering, while the script statement:

LoadingGui.setBitmap(%levelshot); // LoadingGUI type of GuiChunkedBitmapCtrl

had no effect and found out that by the time,

setBitmap()

is actually called on the instance of a GuiChunkedBitmapCtrl control, the value of the %levelshot variable has been owerwritten, with some other string, used inside the script function, that was currently executed.

Since the overwritten value no more points to a valid bitmap the setBitmap() call had no effect.

Here are the details:

My GUI control contains a function to set the controls bitmap:

new GuiChunkedBitmapCtrl(LoadingGui) {
...

function LoadingGui::setInfoDetails(%this, %missionFile)
{
   // Calculate %levelshot location
   ...
   LoadingGui.setBitmap(%levelshot); // This code has no effect
   ...   
}

I started my debug session by checking out the passed filename in the console method of GuiChunkedBitmapCtrl.

ConsoleMethod( GuiChunkedBitmapCtrl, setBitmap, void, 3, 3, "(string filename)"
              "Set the bitmap contained in this control.")
{
   ...
   GuiChunkedBitmapCtrl* ctrl = static_cast<GuiChunkedBitmapCtrl*>( object );

   ctrl->setBitmap( argv[2] ); [b]// All fine here, my bitmap file is passed in via argv[2][/b]
}

The problem seems to be, that the setBitmap() implementation again executes a script by calling the callback onSleep() of the control.

void GuiChunkedBitmapCtrl::setBitmap(const char *name)
{
   bool awake = mAwake;
   if(awake)
      [b]onSleep();[/b]
      
   ...   
}

When stepping through the onSleep() script execution code and constantly back-checking the value of argv[2] (the filename, passed to setBitmap()) on the top of the callstack, I noticed, that the first call to StringStack::setStringValue(const char *s) overwrites the argv[2] value.

That means when GuiChunkedBitmapCtrl::setBitmap() returns from the onSleep() callback, the name variable, used as bitmap name is already damaged.

void GuiChunkedBitmapCtrl::setBitmap(const char *name)
{
   bool awake = mAwake;
   if(awake)
      [b]onSleep();[/b]
   
   [b]// Here the name variable is already damaged   [/b]
   mBitmapName = StringTable->insert(name);
   
   if(awake)
      onWake();
   setUpdate();
}

I solved the problem, by rewriting GuiChunkedBitmapCtrl::setBitmap() in the way GuiBitmapCtrl::setBitmap() is programmed: Copy the value of argv[2] and use the copy later on.

ConsoleMethod( GuiChunkedBitmapCtrl, setBitmap, void, 3, 3, "(string filename)"
              "Set the bitmap contained in this control.")
{
   ...
   GuiChunkedBitmapCtrl* ctrl = static_cast<GuiChunkedBitmapCtrl*>( object );

   char fileName[1024];
   Con::expandScriptFilename(fileName, sizeof(fileName), argv[2]);
   ctrl->setBitmap( fileName );
}

But this seems to be a workaround, since the original code hints to a buffer overflow in the script exectution code, for stacked/recursive script calls.

Has someone seen this before, or can comment on this.

-- Markus