View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002824 | FSSCP | public | 2013-03-26 11:22 | 2013-04-22 11:18 | |
| Reporter | Echelon9 | Assigned To | Echelon9 | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.6.19 | ||||
| Target Version | 3.7.0 | ||||
| Summary | 0002824: AddressSanitizer: global-buffer-overflow in obj_snd_delete_all() objectsnd.cpp:842 | ||||
| Description | Reported by AddressSanitizer, a memory error detector for C/C++, in FS2Open builds based on trunk r9607. ERROR: AddressSanitizer: global-buffer-overflow on address 0x0001038e84c8 at pc 0x10087076c bp 0x7fff5fbfd0b0 sp 0x7fff5fbfd0a8 READ of size 8 at 0x0001038e84c8 thread T0 #0 0x10087076b in std::vector<int, SCP_vm_allocator<int> >::clear stl_vector.h:752 0000001 0x101a32f65 in obj_snd_delete_all objectsnd.cpp:842 0000002 0x101a3303d in obj_snd_level_close objectsnd.cpp:857 0000003 0x1002bac08 in game_level_close freespace.cpp:886 0000004 0x1002bc4ec in freespace_stop_mission freespace.cpp:1077 0000005 0x1002fac86 in game_leave_state freespace.cpp:5627 0000006 0x1008efe23 in gameseq_set_state gamesequence.cpp:275 0000007 0x1002f7769 in game_process_event freespace.cpp:5120 0000008 0x1008f18bf in gameseq_process_events gamesequence.cpp:395 0000009 0x100306978 in game_main freespace.cpp:7033 0000010 0x100307fe6 in SDL_main freespace.cpp:7167 ... // Remove all persistent sounds // void obj_snd_delete_all() { int idx; for(idx=0; idx<MAX_OBJ_SNDS; idx++){ if(Objsnds[idx].flags & OS_USED){ obj_snd_delete_type(Objsnds[idx].objnum); Objects[Objsnds[idx].objnum].objsnd_num.clear(); <=== } } } | ||||
| Steps To Reproduce | 1. Utilise a version of Clang that supports AddressSantizer (https://code.google.com/p/address-sanitizer/wiki/AddressSanitizer). 2. Build with -fsanitize=address C/C++ flag 3. Run the game with mediavps 3.6.12, progress through mission 1 "Surrender, Belisarious", jump out at end of mission. | ||||
| Additional Information | ERROR: AddressSanitizer: global-buffer-overflow on address 0x0001038e84c8 at pc 0x10087076c bp 0x7fff5fbfd0b0 sp 0x7fff5fbfd0a8 READ of size 8 at 0x0001038e84c8 thread T0 #0 0x10087076b in std::vector<int, SCP_vm_allocator<int> >::clear stl_vector.h:752 0000001 0x101a32f65 in obj_snd_delete_all objectsnd.cpp:842 0000002 0x101a3303d in obj_snd_level_close objectsnd.cpp:857 0000003 0x1002bac08 in game_level_close freespace.cpp:886 0000004 0x1002bc4ec in freespace_stop_mission freespace.cpp:1077 0000005 0x1002fac86 in game_leave_state freespace.cpp:5627 0000006 0x1008efe23 in gameseq_set_state gamesequence.cpp:275 0000007 0x1002f7769 in game_process_event freespace.cpp:5120 0000008 0x1008f18bf in gameseq_process_events gamesequence.cpp:395 0000009 0x100306978 in game_main freespace.cpp:7033 0000010 0x100307fe6 in SDL_main freespace.cpp:7167 #11 0x10000295a in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300 0000012 0x7fff8ffe3ed9 in _CFXNotificationPost (in CoreFoundation) + 2553 0000013 0x7fff85fc6e25 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63 0000014 0x7fff8b92855c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291 0000015 0x7fff8b928295 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215 0000016 0x7fff8b925481 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565 0000017 0x7fff8b92507b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350 0000018 0x7fff85fe070a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307 0000019 0x7fff85fe056c in _NSAppleEventManagerGenericHandler (in Foundation) + 105 0000020 0x7fff8e3a9077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306 0000021 0x7fff8e3a8ed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36 0000022 0x7fff8e3a8d98 in aeProcessAppleEvent (in AE) + 317 0000023 0x7fff88b11708 in AEProcessAppleEvent (in HIToolbox) + 99 0000024 0x7fff8b921865 in _DPSNextEvent (in AppKit) + 1455 0000025 0x7fff8b920e21 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127 0000026 0x7fff8b9181d2 in -[NSApplication run] (in AppKit) + 516 0000027 0x100004a73 in CustomApplicationMain SDLMain.m:227 0000028 0x1000044f0 in main SDLMain.m:377 0000029 0x1000016c3 in start (in FS2_Open (debug)) + 51 0000030 0x0 in 0x0 0x0001038e84c8 is located 56 bytes to the left of global variable 'Objects' from 'fs2open/trunk/fs2_open/projects/Xcode4/../../code/object/object.cpp' (0x1038e8500) of size 1904000 0x0001038e84c8 is located 0 bytes to the right of global variable 'Viewer_obj' from 'fs2open/trunk/fs2_open/projects/Xcode4/../../code/object/object.cpp' (0x1038e84c0) of size 8 'Viewer_obj' is ascii string '' Shadow bytes around the buggy address: 0x10002071d040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d080: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 =>0x10002071d090: 00 f9 f9 f9 f9 f9 f9 f9 00[f9]f9 f9 f9 f9 f9 f9 0x10002071d0a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d0b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d0c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d0d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002071d0e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap righ redzone: fb Freed Heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 ASan internal: fe ==59088==ABORTING | ||||
| Tags | No tags attached. | ||||
|
|
Prior to calling obj_snd_delete_type(), Objsnds[idx].objnum = 0, on the first run. Subsequent to which, Objsnds[idx].objnum has been set to -1. Using this negative index into the Objects[] array on the next line is the cause of the crash. |
|
|
fix-mantis-2824-objectsndnum.patch (401 bytes)
Index: code/object/objectsnd.cpp
===================================================================
--- code/object/objectsnd.cpp (revision 9630)
+++ code/object/objectsnd.cpp (working copy)
@@ -838,8 +838,6 @@
for(idx=0; idx<MAX_OBJ_SNDS; idx++){
if(Objsnds[idx].flags & OS_USED){
obj_snd_delete_type(Objsnds[idx].objnum);
-
- Objects[Objsnds[idx].objnum].objsnd_num.clear();
}
}
}
|
|
|
Ready for code review. The problem is that within obj_snd_delete_type() an iterator passes through the STL vector, and with only a few exceptions, clears each item via obj_snd_delete(). What is preventing us from doing a wholesale .clear() as was attempted here is that obj_snd_delete_type() is used in other spots in a more targeted way for certain types of sounds only. It can't be easily abstracted to a wholesale clear within that function. I think for now it is best that we just rely upon our own internally written clearing mechanism. |
|
|
looks good to me |
|
|
Fix committed to trunk@9644. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-03-26 11:22 | Echelon9 | New Issue | |
| 2013-03-27 12:22 | Echelon9 | Note Added: 0014847 | |
| 2013-03-27 12:22 | Echelon9 | Status | new => confirmed |
| 2013-04-14 01:07 | Echelon9 | Assigned To | => Echelon9 |
| 2013-04-14 01:07 | Echelon9 | Status | confirmed => assigned |
| 2013-04-15 14:07 | Echelon9 | File Added: fix-mantis-2824-objectsndnum.patch | |
| 2013-04-15 14:10 | Echelon9 | Note Added: 0014926 | |
| 2013-04-15 14:10 | Echelon9 | Assigned To | Echelon9 => chief1983 |
| 2013-04-15 14:10 | Echelon9 | Status | assigned => code review |
| 2013-04-22 08:03 | niffiwan | Note Added: 0014949 | |
| 2013-04-22 11:18 | Echelon9 | Assigned To | chief1983 => Echelon9 |
| 2013-04-22 11:18 | Echelon9 | Status | code review => assigned |
| 2013-04-22 11:18 | Echelon9 | Changeset attached | => fs2open trunk r9644 |
| 2013-04-22 11:18 | Echelon9 | Note Added: 0014950 | |
| 2013-04-22 11:18 | Echelon9 | Status | assigned => resolved |
| 2013-04-22 11:18 | Echelon9 | Resolution | open => fixed |
