nsis 2.0 alpha 7 bug

Hi all,

On my machine running Windows XP nsis dies when launched.

I've tracked it down to this:

// Added by Ximon Eighteen 5th August 2002
#ifdef NSIS_CONFIG_PLUGIN_SUPPORT
void CEXEBuild::build_plugin_table(void)
{
plugin_used = false;
uninst_plugin_used = false;
char* nsisdir = definedlist.find("NSISDIR");
if (nsisdir)
{
char* searchPath = new char [strlen(nsisdir)+6];
if (searchPath)
{
wsprintf(searchPath,"%s\\plugins",nsisdir);
INFO_MSG("\nProcessing plugin dlls: \"%s\\*.dll\"\n",searchPath);
m_plugins.FindCommands(searchPath,display_info?true:false);
delete[] searchPath; // <<<<< THIS IS THE LINE
}
}
}

I've seen that operator delete is redefined in ResourceEditor.cpp using malloc/free functions, and no operator new[] and operator delete[] is defined ... this doesn't seem good.

Ofcorse if I undefine the NSIS_CONFIG_PLUGIN_SUPPORT everything seams to be ok.

I'm posting this here because I don't have a sourceforge account, yet.

Best regards,
Cristian.

Sounds like you may have found whats killing my installation of NSIS at work (my home copy works fine so this has been confusing me)... KiCHiK -- you there (hint hint) ? 😁

Ah! The allocated memory is just a little bit too small.

Reason:
-------

char* searchPath = new char [strlen(nsisdir)+6];
...
wsprintf(searchPath,"%s\\plugins",nsisdir);

You're copying more bytes then allocated before, thus you overwrite memory that could belong to anything else.

Historical analysis:
--------------------

Plugins were stored in \bin, but now in \plugins, which is 4 bytes longer. Apparently someone did change the code without checking for sideeffects 😱 Shame on you! 😁

Conclusion:
-----------

The new line should be like this:
char* searchPath = new char [strlen(nsisdir)+10];

This bug is most likely the reason for all the crashes some people still get. I would advise to check for similar bugs.

Ah not my bug then :P (cool)

kickik made a bugfix in the CVS version a few weeks ago. But he changed it to 9. 🧟

// Added by Ximon Eighteen 5th August 2002

#ifdef NSIS_CONFIG_PLUGIN_SUPPORT

void CEXEBuild::build_plugin_table(void)

{

  plugin_used = false;

  uninst_plugin_used = false;

  char* nsisdir = definedlist.find("NSISDIR");

  if (nsisdir)

  {

    char* searchPath = new char [strlen(nsisdir)+9];

    if (searchPath)

    {

      wsprintf(searchPath,"%s\\plugins",nsisdir);

      INFO_MSG("\nProcessing plugin dlls: \"%s\\*.dll\"\n",searchPath);

      m_plugins.FindCommands(searchPath,display_info?true:false);

      delete[] searchPath;

    }

  }

}

Hmm..which value is the right one?

"%s\\plugins",nsisdir -- so space required should be strlen(nsisdir)+one for the \\ (which is actually just \) and then 7 for "plugins" and another one for the null terminator..
i.e. strlen(nsisdir)+9.

It should be nine (\\ - 1, p - 2, l - 3, u - 4, g - 5, i - 6, n - 7, s - 8, \0 - 9). Rainwater made the bug, and eccles fixed it.

And as for the original "solution", I have checked this when I added it. It does use my new operator even with new[].

"plugins" was "bin" before. Therefore, when I changed it to plugins, the allocation was incorrect.