Source Code Project Mantis - FSSCP
View Issue Details
0001939FSSCPgameplaypublic2009-06-12 21:032010-01-02 03:31
ReporterRedDwarf 
Assigned ToEchelon9 
PrioritynormalSeveritycrashReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.6.11 
Target VersionFixed in Version3.6.10 
Summary0001939: Buffer Overflow
DescriptionCompile the game with "fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g"
...I don't remember exactly how this works, but I think either "-D_FORTIFY_SOURCE=2" or "-fstack-protector" enables additional runtime buffer overflow checks. Also, the crash doesn't happens with "-O0".

The thing is that the game crashes at start (you do not have an opportunity to do anything) when using the fsport 3.1.1 with a "*** buffer overflow detected ***" and a memory map.
These are the MD5SUMs:
7947b84563bd538ea68b5866cf8134f0 fsport3_1_1.vp
009e837b93ddf50589da03e5045940b5 fsport-missions.vp
The game is executed with only "-mod fsport".
This is with fs2_open from tag "fs2_open_3_6_10", revision 5210.

A "__strcat_chk" is who finds the problem. It seems to happen at "required_string("$SBank Capacity:");" in ship/ship.cpp:3977 when parse_error_text has this: "\nin ship: Terran NavBuoy#destroyable\000s default secondary banks\000".
TagsNo tags attached.
Attached Files

Notes
(0010974)
RedDwarf   
2009-06-12 22:44   
Ok, was easier than that... trying to debug optimized code provokes these things :-p

The problem is in the strcat after required_string, in ship/ship.cpp:3978.
parse_error_text is defined as "char parse_error_text[64]". And the line strcat(parse_error_text,"'s secondary banks capacities"); tries to concatenate a 36 bytes long parse_error_text ("\nin ship: Terran NavBuoy#destroyable") with the 29 bytes long "'s secondary banks capacities"... 66 bytes in a 64 bytes array.
(0010978)
portej05   
2009-06-15 21:40   
I'll mark this as acknowledged - I can see exactly what's going on here :P
parse_error_text as 64 bytes seems just a little on the small side.
Since we're mainly using stack allocated memory, I've put together a small set of the str*_s functions (so that cross platform compiling still works) to try and help track down these problems.
My guess is that this isn't the only case where this happens!
(0011479)
Echelon9   
2010-01-02 03:31   
portej05's safe_strings and an increase in parse_error_text[] should prevent this reoccurring.

Issue History
2009-06-12 21:03RedDwarfNew Issue
2009-06-12 22:44RedDwarfNote Added: 0010974
2009-06-15 21:40portej05Note Added: 0010978
2009-06-15 21:40portej05Statusnew => acknowledged
2010-01-02 03:31Echelon9Note Added: 0011479
2010-01-02 03:31Echelon9Statusacknowledged => resolved
2010-01-02 03:31Echelon9Fixed in Version => 3.6.10
2010-01-02 03:31Echelon9Resolutionopen => fixed
2010-01-02 03:31Echelon9Assigned To => Echelon9