Am Fri, 18 Jan 2019 01:13:08 +0200
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
Hi,
On 1/17/19 9:17 PM, Christian Zietz wrote:
for quite some time I have been puzzled why Hatari (snapshot builds
for Windows from antarctica.no) sometimes worked and sometimes
crashed with an HEAP_CORRUPTION error directly after start. Today,
I finally figured out that it works if I start it from the Windows
command line with an absolute path to the executable, but crashes
if I start it with just a relative path to the executable.
With that knowledge and a debugger I found the reason: a buffer
overflow in Paths_Init(). When started from cmd.exe giving a
relative path to the executable, psExecDir [1] will also only
contain a relative path. Notice how in [2] only just enough memory
is allocated for sDataDir to hold psExecDir, the path separator
("\" under Windows) and BIN2DATADIR, which is "." for the Windows
build.
However, File_MakeAbsoluteName, called from [3], will in the end
write back the full, absolute filename into that buffer [4], thereby
overflowing it.
Ouch.
Since Windows 10 has very good checkers for heap
corruption (e.g. by buffer overflow), it then almost immediately
terminates the offending process.
I've always though Linux has better / more accessible ones, but
strangely enough e.g. Valgrind doesn't complain about this.
The bug normally does not happen on Linux: Paths_InitExecDir() returns
an absolute path here since we can read the location with readlink().
Solution: Always allocate a sufficiently sized buffer
(FILENAME_MAX+1?) for sDataDir.
I think FILENAME_MAX is supposed to include the terminating nil, as
that's also specified part of the filename (given to Linux system
calls).
Right.
Attached is patch for you to test.
Please also check the return code of malloc for NULL (or use
Str_Alloc() to get the memory), otherwise static analyzers will
complain again.
Thanks,
Thomas