Bug #1307: rb_w32_spawn broken - builds fail

• Status: Closed
• Assignee: -
• Target version: 1.9.1
• ruby -v: Latest trunk

[cfis (Charlie Savage)]

=begin
The recent change to rb_w32_spawn have totally broken it. First, it no longer quotes program names correctly. Second, it causes a buffer overflow corrupting the stack, causing the build to fail. And third, it may leak memory (not sure on that one).

Compiling with VC (-RTC1), when nmake gets to the big decimal extension it calls rb_w32_spawn. The parameters are:

cmd is cl -nologo -Feconftest -I../../.ext/include/i386-mswin32_90 -I../.././../include -I../.././../ext/bigdecimal -I../.././../include -I. -I./.. -I./../missing -DLIBRUBY_SO="msvcr90d-ruby191.dll" -IC:\Development\msvc\include -nologo -MDd -Zi -RTC1 -W3 -wd4996 -Od -Zm600 conftest.c msvcr90d-ruby191-static.lib unicows.lib oldnames.lib user32.lib advapi32.lib shell32.lib ws2_32.lib -link -nologo -incremental:no -nologo -debug -opt:ref -opt:icf -libpath:C:\Development\msvc\lib -libpath:"." -libpath:"../.."

prog is NULL

mode is ONE

Then at line 1074:

len is 2

fbuf is C:\Development\Microsoft Visual Studio 9.0\VC\BIN/cl.exe

prog is -nologo -Feconftest -I../../.ext/include/i386-mswin32_90 -I../.././../include -I../.././../ext/bigdecimal -I../.././../include -I. -I./.. -I./../missing -DLIBRUBY_SO="msvcr90d-ruby191.dll" -IC:\Development\msvc\include -nologo -MDd -Zi -RTC1 -W3 -wd4996 -Od -Zm600 conftest.c msvcr90d-ruby191-static.lib unicows.lib oldnames.lib user32.lib advapi32.lib shell32.lib ws2_32.lib const char *

Then comes this code:

len += strlen(prog) + (quote ? 2 : 0) + 1;
cmd = p = ALLOCA_N(char, len);
if (quote) *p++ = '"';
p += strlcpy(p, fbuf, --len);
if (quote) *p++ = '"';
p += strlcpy(p, prog, --len);

cmd is not big enough to fbuf and prog - so here is the buffer overflow. That then will kill the program.

Second, once all the processing is done, cmd is:

C:\Development\Microsoft Visual Studio 9.0\VC\BIN/cl.exe -nologo -Feconftest -I../../.ext/include/i386-mswin32_90 -I../.././../include -I../.././../ext/bigdecimal -I../.././../include -I. -I./.. -I./../missing -DLIBRUBY_SO="msvcr90d-ruby191.dll" -IC:\Development\msvc\include -nologo -MDd -Zi -RTC1 -W3 -wd4996 -Od -Zm600 conftest.c msvcr90d-ruby191-static.lib unicows.lib oldnames.lib user32.lib advapi32.lib shell32.lib ws2_32.lib -link -nologo -incremental:no -nologo -debug -opt:ref -opt:icf -libpath:C:\Development\msvc\lib -libpath:"." -libpath:"../.."

But notice the spaces in the path. The program names should be quoted (but is not). In fact, it seems to me program names always should be quoted.

Last, where exactly is the cmd buffer deallocated? That looks like a potential memory leak.

Altogether, this makes bug causes trunk to fail to build on Windows with VC2008.
=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Sun, 22 Mar 2009 16:09:48 +0900,
Charlie Savage wrote in [ruby-core:22988]:

Author: Charlie Savage
Status: Open, Priority: High
Target version: 1.9.1
ruby -v: Latest trunk

What is the revision?

I think it's fixed at r23031 already.

--
Nobu Nakada

=end

[cfis (Charlie Savage)]

=begin
Hi Nobu,

I tested r23017 (ok), r23023 (breaks here), r23025 (still broken) and r23031 (still broken). So the error is still occurring. The data I pasted from the bug report above was generated using r23031.

Let me know if you need any more information...

Charlie
=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Mon, 23 Mar 2009 07:22:35 +0900,
Charlie Savage wrote in [ruby-core:22996]:

I tested r23017 (ok), r23023 (breaks here), r23025 (still
broken) and r23031 (still broken). So the error is still
occurring. The data I pasted from the bug report above was
generated using r23031.

Then why did you cite the code from r23025?

--
Nobu Nakada

=end

[cfis (Charlie Savage)]

=begin
Hi Nobu,

Ah, you are right. The latest revision does work for me. Sorry for the noise - my mistake.

Charlie
=end

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

=begin

=end