gargoyle-alan3 crashes when run without arguments

[GoogleCodeExporter]

This is occurs for me on 64-bit Linux (Gentoo, GCC 4.3.3) with svn revision
219 of Gargoyle.

To reproduce:
 - open a terminal window and run "gargoyle-alan3"

Expected behaviour:
 - an X window displaying an error message along the lines of "Can't find a
game file..."

Observed behaviour:
 - application crashes on a segmentation fault

gargoyle-alan2 does behave as expected. It's probably trivial to fix, but I
thought I'd report it anyway or it might go unnoticed.

Original issue reported on code.google.com by znxfire...@gmail.com on 20 Jun 2009 at 9:18

[GoogleCodeExporter]

Thanks for the report.  I can't reproduce this issue on my system.  Possibly 
you have
an older version of gargoyle in the path?

Compiling the source from svn should result in binaries without the "gargoyle-"
prefix, unless you've taken steps to change that.

Original comment by bcressey@gmail.com on 22 Jun 2009 at 3:58

[GoogleCodeExporter]

Actually, the problem is related to symlinks. I use a custom Gentoo ebuild to 
install
the binaries into /usr/libexec/gargoyle/ and then add symlinks with the
gargoyle-prefix to /bin (to avoid name clashes with other applications, in 
particular
the Git VCS tool).

The actual bug seems to be in openResourceFile() (defined in glkstart.c) which
assumes the game file path contains a period. If it doesn't, strrchr() returns 
NULL
and the strcpy a few lines later writes to address 0 causing a segmentation 
fault.

The reason this bug manifests itself when called through a symlink is because in
unixargs.c there is some code that sets the game file path to the executable 
file
path (if there is no other argument that specifies this) but this path usually
doesn't contain a period. Furthermore, this is done only if the executable path 
does
not end with "/alan3", which is why calling the executable directly does work!

So I suggest openResourceFile() be updated to check if extension != NULL and
strlen(extension) >= 4 (otherwise the strcpy still overflows the buffer if the
original file extension is less than 3 characters long).

Original comment by znxfire...@gmail.com on 22 Jun 2009 at 5:07

[GoogleCodeExporter]

Oh, and a simple way to reproduce it:
$ cp alan3 /tmp/foo
$ /tmp/foo
Segmentation fault!

Original comment by znxfire...@gmail.com on 22 Jun 2009 at 5:09

[GoogleCodeExporter]

Original comment by bcressey@gmail.com on 6 Jul 2009 at 9:16

• Changed state: Accepted
• Added labels: OpSys-All

[GoogleCodeExporter]

Fixed in r241.

I removed the offending lines in openResourceFile(), as the extension string was
never actually used and would have been difficult to construct properly at that 
point
in the code.

I also changed the logic for the program name matching for alan2 and alan3, so 
that
running gargoyle-alan? will also trigger the usage prompt.

Running your test code now results in an ACODE error, which I think is 
acceptable. 
My reading of the args code suggests that it's intended to support the case 
where the
story file is executed directly, perhaps by wrapping both the interpreter and 
the
game in a single file.  I don't see how to tweak it further without disabling 
this
functionality, however limited it may be in practice.

Original comment by bcressey@gmail.com on 20 Jul 2009 at 6:17

• Changed state: Fixed