View Issue Details

IDProjectCategoryView StatusLast Update
0002834FSSCPpublic2013-04-22 11:21
ReporterEchelon9 Assigned ToEchelon9  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.19 
Target Version3.7.0 
Summary0002834: AddressSanitizer: global-buffer-overflow in get_max_ammo_count_for_bank() ship.cpp:15249
DescriptionReported by AddressSanitizer, a memory error detector for C/C++, in FS2Open builds based on trunk r9617.

ERROR: AddressSanitizer: global-buffer-overflow on address 0x0001049b1c40 at pc 0x1020be74a bp 0x7fff5fbfb230 sp 0x7fff5fbfb228
READ of size 4 at 0x0001049b1c40 thread T0
    #0 0x1020be749 in get_max_ammo_count_for_bank ship.cpp:15249
    0000001 0x1020baa3a in ship_process_post ship.cpp:8198
    0000002 0x101a5a5ca in obj_move_all_post object.cpp:1226
    0000003 0x101a5e6f2 in obj_move_all object.cpp:1445
    0000004 0x1002ea2fd in game_simulation_frame freespace.cpp:3987
    0000005 0x1002eef51 in game_frame freespace.cpp:4380
    0000006 0x1002f473b in game_do_frame freespace.cpp:4791
    0000007 0x10030029a in game_do_state freespace.cpp:6466
    0000008 0x1008f1b89 in gameseq_process_events gamesequence.cpp:405
    0000009 0x100306b18 in game_main freespace.cpp:7033
    0000010 0x100308186 in SDL_main freespace.cpp:7167
    ...

/**
 * Determine the number of secondary ammo units (missile/bomb) allowed max for a ship
 */
int get_max_ammo_count_for_bank(int ship_class, int bank, int ammo_type)
{
    float capacity, size;

    capacity = (float) Ship_info[ship_class].secondary_bank_ammo_capacity[bank];
    size = (float) Weapon_info[ammo_type].cargo_size; <========
    return (int) (capacity / size);
}
Steps To Reproduce1. 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 to flight in red alert mission 'Apocalypse'
Additional InformationERROR: AddressSanitizer: global-buffer-overflow on address 0x0001049b1c40 at pc 0x1020be74a bp 0x7fff5fbfb230 sp 0x7fff5fbfb228
READ of size 4 at 0x0001049b1c40 thread T0
    #0 0x1020be749 in get_max_ammo_count_for_bank ship.cpp:15249
    0000001 0x1020baa3a in ship_process_post ship.cpp:8198
    0000002 0x101a5a5ca in obj_move_all_post object.cpp:1226
    0000003 0x101a5e6f2 in obj_move_all object.cpp:1445
    0000004 0x1002ea2fd in game_simulation_frame freespace.cpp:3987
    0000005 0x1002eef51 in game_frame freespace.cpp:4380
    0000006 0x1002f473b in game_do_frame freespace.cpp:4791
    0000007 0x10030029a in game_do_state freespace.cpp:6466
    0000008 0x1008f1b89 in gameseq_process_events gamesequence.cpp:405
    0000009 0x100306b18 in game_main freespace.cpp:7033
    0000010 0x100308186 in SDL_main freespace.cpp:7167
    #11 0x10000287a 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 0x100004993 in CustomApplicationMain SDLMain.m:227
    0000028 0x100004410 in main SDLMain.m:377
    0000029 0x1000015e3 in start (in FS2_Open (debug)) + 51
    0000030 0x0 in 0x0
0x0001049b1c40 is located 259072 bytes to the right of global variable 'Weapons' from 'fs2open/trunk/fs2_open/projects/Xcode4/../../code/weapon/weapons.cpp' (0x10482a640) of size 1344000
Shadow bytes around the buggy address:
  0x100020936330: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x100020936340: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x100020936350: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x100020936360: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x100020936370: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
=>0x100020936380: f9 f9 f9 f9 f9 f9 f9 f9[f9]f9 f9 f9 f9 f9 f9 f9
  0x100020936390: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209363a0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209363b0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209363c0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209363d0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
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
==89431==ABORTING
TagsNo tags attached.

Relationships

related to 0002821 resolvedEchelon9 AddressSanitizer: global-buffer-overflow in ship_get_secondary_weapon_range() ship.cpp:15206 

Activities

Echelon9

2013-04-19 10:27

developer  

fix-mantis-2834_get_max_ammo_count_for_bank.patch (715 bytes)   
Index: ship/ship.cpp
===================================================================
--- ship/ship.cpp	(revision 9638)
+++ ship/ship.cpp	(working copy)
@@ -15244,10 +15251,14 @@
 int get_max_ammo_count_for_bank(int ship_class, int bank, int ammo_type)
 {
 	float capacity, size;
-
-	capacity = (float) Ship_info[ship_class].secondary_bank_ammo_capacity[bank];
-	size = (float) Weapon_info[ammo_type].cargo_size;
-	return (int) (capacity / size);
+    
+	if (ship_class < 0 || bank < 0 || ammo_type < 0) {
+		return 0;
+	} else {
+		capacity = (float) Ship_info[ship_class].secondary_bank_ammo_capacity[bank];
+		size = (float) Weapon_info[ammo_type].cargo_size;
+		return (int) (capacity / size);
+	}
 }
 
 /**

Echelon9

2013-04-19 10:28

developer   ~0014933

Ready for code review.
This was another case of -1 being passed as an array index.

niffiwan

2013-04-22 06:04

developer   ~0014947

looks good to me

Echelon9

2013-04-22 11:21

developer   ~0014951

Fix committed to trunk@9645.

Related Changesets

fs2open: trunk r9645

2013-04-22 08:12

Echelon9


Ported: N/A

Details Diff
Fix Mantis 2834: AddressSanitizer: global-buffer-overflow in get_max_ammo_count_for_bank() Affected Issues
0002834
mod - /trunk/fs2_open/code/ship/ship.cpp Diff File

Issue History

Date Modified Username Field Change
2013-04-03 12:13 Echelon9 New Issue
2013-04-03 12:15 Echelon9 Relationship added related to 0002821
2013-04-13 12:48 Echelon9 Assigned To => Echelon9
2013-04-13 12:48 Echelon9 Status new => assigned
2013-04-19 10:27 Echelon9 File Added: fix-mantis-2834_get_max_ammo_count_for_bank.patch
2013-04-19 10:28 Echelon9 Note Added: 0014933
2013-04-19 10:28 Echelon9 Assigned To Echelon9 => chief1983
2013-04-19 10:28 Echelon9 Status assigned => code review
2013-04-22 06:04 niffiwan Note Added: 0014947
2013-04-22 11:21 Echelon9 Assigned To chief1983 => Echelon9
2013-04-22 11:21 Echelon9 Status code review => assigned
2013-04-22 11:21 Echelon9 Changeset attached => fs2open trunk r9645
2013-04-22 11:21 Echelon9 Note Added: 0014951
2013-04-22 11:21 Echelon9 Status assigned => resolved
2013-04-22 11:21 Echelon9 Resolution open => fixed