2019-12-05 22:11 EST


View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0001939FSSCPgameplaypublic2010-01-02 03:31
ReporterRedDwarf 
Assigned ToEchelon9 
PrioritynormalSeveritycrashReproducibilityalways
StatusresolvedResolutionfixed 
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

-Relationships
+Relationships

-Notes

~0010974

RedDwarf (reporter)

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 (reporter)

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 (developer)

portej05's safe_strings and an increase in parse_error_text[] should prevent this reoccurring.
+Notes

-Issue History
Date Modified Username Field Change
2009-06-12 21:03 RedDwarf New Issue
2009-06-12 22:44 RedDwarf Note Added: 0010974
2009-06-15 21:40 portej05 Note Added: 0010978
2009-06-15 21:40 portej05 Status new => acknowledged
2010-01-02 03:31 Echelon9 Note Added: 0011479
2010-01-02 03:31 Echelon9 Status acknowledged => resolved
2010-01-02 03:31 Echelon9 Fixed in Version => 3.6.10
2010-01-02 03:31 Echelon9 Resolution open => fixed
2010-01-02 03:31 Echelon9 Assigned To => Echelon9
+Issue History