All Projects FSSCP  wxFRED

View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002896FSSCPpublic2013-06-30 05:172013-12-07 05:18
ReporterEchelon9 Assigned ToEchelon9  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionreopened 
Product Version3.7.0 RC2 
Target Version3.7.0 
Summary0002896: AddressSanitizer: global-buffer-overflow in hud_lock_reset() hudlock.cpp
DescriptionReported by AddressSanitizer, a memory error detector for C/C++, in FS2Open builds based on trunk r9698 + BP.patch

ERROR: AddressSanitizer: global-buffer-overflow on address 0x000104f44cc0 at pc 0x100d3dec1 bp 0x7fff5fbfbdd0 sp 0x7fff5fbfbdc8
READ of size 4 at 0x000104f44cc0 thread T0
    #0 0x100d3dec0 in hud_lock_reset hudlock.cpp:307
    0000001 0x100b0541a in hud_target_change_check hudtarget.cpp:4446
    0000002 0x100de82a0 in hud_update_frame hud.cpp:1468
    0000003 0x1002f5af0 in game_simulation_frame freespace.cpp:4149
    0000004 0x1002fa410 in game_frame freespace.cpp:4504
    0000005 0x1002ffc2b in game_do_frame freespace.cpp:4915
    0000006 0x10030b786 in game_do_state freespace.cpp:6591
    0000007 0x1009150f8 in gameseq_process_events gamesequence.cpp:405
    0000008 0x100312026 in game_main freespace.cpp:7158
    ...

// Reset data used for player lock indicator
void hud_lock_reset(float lock_time_scale)
{
    weapon_info *wip;
    ship_weapon *swp;

    swp = &Player_ship->weapons;
    wip = &Weapon_info[swp->secondary_bank_weapons[swp->current_secondary_bank]];

    Player_ai->current_target_is_locked = 0;
    Players[Player_num].lock_indicator_visible = 0;
    Player->target_in_lock_cone = 0;
    Player->lock_time_to_target = i2fl(wip->min_lock_time*lock_time_scale); <=======
    Player->current_target_sx = -1;
    Player->current_target_sy = -1;
    Player->locking_subsys=NULL;
    Player->locking_on_center=0;
    Player->locking_subsys_parent=-1;
    hud_stop_looped_locking_sounds();

    Lock_gauge_draw_stamp = -1;
    Lock_gauge_draw = 0;

    // reset the lock anim time elapsed
    Lock_anim.time_elapsed = 0.0f;
}
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, and Blue Plant WiH2. Start The Blade Itself, report should occur straight away.
Additional InformationERROR: AddressSanitizer: global-buffer-overflow on address 0x000104f44cc0 at pc 0x100d3dec1 bp 0x7fff5fbfbdd0 sp 0x7fff5fbfbdc8
READ of size 4 at 0x000104f44cc0 thread T0
    #0 0x100d3dec0 in hud_lock_reset hudlock.cpp:307
    0000001 0x100b0541a in hud_target_change_check hudtarget.cpp:4446
    0000002 0x100de82a0 in hud_update_frame hud.cpp:1468
    0000003 0x1002f5af0 in game_simulation_frame freespace.cpp:4149
    0000004 0x1002fa410 in game_frame freespace.cpp:4504
    0000005 0x1002ffc2b in game_do_frame freespace.cpp:4915
    0000006 0x10030b786 in game_do_state freespace.cpp:6591
    0000007 0x1009150f8 in gameseq_process_events gamesequence.cpp:405
    0000008 0x100312026 in game_main freespace.cpp:7158
    0000009 0x1003136c4 in SDL_main freespace.cpp:7292
    0000010 0x100003393 in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300
    #11 0x7fff87ce4ed9 in _CFXNotificationPost (in CoreFoundation) + 2553
    0000012 0x7fff9079e7b5 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63
    0000013 0x7fff8a09752c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291
    0000014 0x7fff8a097265 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215
    0000015 0x7fff8a094451 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565
    0000016 0x7fff8a09404b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350
    0000017 0x7fff907b807a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307
    0000018 0x7fff907b7edc in _NSAppleEventManagerGenericHandler (in Foundation) + 105
    0000019 0x7fff8bfa7077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306
    0000020 0x7fff8bfa6ed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36
    0000021 0x7fff8bfa6d98 in aeProcessAppleEvent (in AE) + 317
    0000022 0x7fff8b7f2708 in AEProcessAppleEvent (in HIToolbox) + 99
    0000023 0x7fff8a090835 in _DPSNextEvent (in AppKit) + 1455
    0000024 0x7fff8a08fdf1 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127
    0000025 0x7fff8a0871a2 in -[NSApplication run] (in AppKit) + 516
    0000026 0x100005502 in CustomApplicationMain SDLMain.m:227
    0000027 0x100004f6f in main SDLMain.m:377
    0000028 0x1000020b3 in start (in FS2_Open (debug)) + 51
    0000029 0x0 in 0x0
0x000104f44cc0 is located 259136 bytes to the right of global variable 'Weapons' from 'fs2_open/projects/Xcode4/../../code/weapon/weapons.cpp' (0x104dbd680) of size 1344000
Shadow bytes around the buggy address:
  0x1000209e8940: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e8950: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e8960: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e8970: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e8980: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
=>0x1000209e8990: f9 f9 f9 f9 f9 f9 f9 f9[f9]f9 f9 f9 f9 f9 f9 f9
  0x1000209e89a0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e89b0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e89c0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e89d0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x1000209e89e0: 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 right 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
==86489==ABORTING
TagsNo tags attached.

Activities

Echelon9

2013-06-30 05:33

developer  

fix-mantis-2896.patch (892 bytes)    
Index: code/hud/hudlock.cpp
===================================================================
--- code/hud/hudlock.cpp	(revision 9698)
+++ code/hud/hudlock.cpp	(working copy)
@@ -299,12 +299,17 @@
 	ship_weapon	*swp;
 
 	swp = &Player_ship->weapons;
-	wip = &Weapon_info[swp->secondary_bank_weapons[swp->current_secondary_bank]];
+    
+	if (swp->current_secondary_bank > 0) {
+		wip = &Weapon_info[swp->secondary_bank_weapons[swp->current_secondary_bank]];
+		Player->lock_time_to_target = i2fl(wip->min_lock_time*lock_time_scale);
+	} else {
+		Player->lock_time_to_target = 0.0f;
+	}
 
 	Player_ai->current_target_is_locked = 0;
 	Players[Player_num].lock_indicator_visible = 0;
 	Player->target_in_lock_cone = 0;
-	Player->lock_time_to_target = i2fl(wip->min_lock_time*lock_time_scale);
 	Player->current_target_sx = -1;
 	Player->current_target_sy = -1;
 	Player->locking_subsys=NULL;
fix-mantis-2896.patch (892 bytes)   

Echelon9

2013-06-30 05:34

developer   ~0015149

See patch attached for proposed resolution.

niffiwan

2013-06-30 07:59

developer   ~0015150

Looks good to me.

Echelon9

2013-06-30 11:18

developer   ~0015151

Fix committed to trunk@9699.

Echelon9

2013-12-07 04:41

developer   ~0015509

AddressSanitizer reports a further global-buffer-overflow within this function in Diaspora training missions using r10201.

// Reset data used for player lock indicator
void hud_lock_reset(float lock_time_scale)
{
    weapon_info *wip;
    ship_weapon *swp;

    swp = &Player_ship->weapons;
    
    if (swp->current_secondary_bank >= 0) {
        Assert(swp->current_secondary_bank < MAX_SHIP_SECONDARY_BANKS);
        wip = &Weapon_info[swp->secondary_bank_weapons[swp->current_secondary_bank]];
        Player->lock_time_to_target = i2fl(wip->min_lock_time*lock_time_scale); <===== HERE
    } else {
        Player->lock_time_to_target = 0.0f;
    }

Echelon9

2013-12-07 05:18

developer   ~0015510

Fix committed to trunk@10204.

Related Changesets

fs2open: trunk r9699

2013-06-30 08:21

Echelon9


Ported: N/A

Details Diff		 Fix Mantis 2896: AddressSanitizer: global-buffer-overflow in hud_lock_reset() hudlock.cpp Affected Issues
0002896
mod - /trunk/fs2_open/code/hud/hudlock.cpp Diff File

fs2open: trunk r10204

2013-12-07 00:49

Echelon9


Ported: N/A

Details Diff		 Fix Mantis 2896: AddressSanitizer: global-buffer-overflow in hud_lock_reset() hudlock.cpp (v2) Affected Issues
0002896
mod - /trunk/fs2_open/code/hud/hudlock.cpp Diff File

Issue History

Date Modified Username Field Change
2013-06-30 05:17 Echelon9 New Issue
2013-06-30 05:17 Echelon9 Status new => assigned
2013-06-30 05:17 Echelon9 Assigned To => Echelon9
2013-06-30 05:33 Echelon9 File Added: fix-mantis-2896.patch
2013-06-30 05:34 Echelon9 Status assigned => code review
2013-06-30 05:34 Echelon9 Note Added: 0015149
2013-06-30 07:59 niffiwan Note Added: 0015150
2013-06-30 11:18 Echelon9 Changeset attached => fs2open trunk r9699
2013-06-30 11:18 Echelon9 Note Added: 0015151
2013-06-30 11:18 Echelon9 Status code review => resolved
2013-06-30 11:18 Echelon9 Resolution open => fixed
2013-12-07 04:41 Echelon9 Note Added: 0015509
2013-12-07 04:41 Echelon9 Status resolved => feedback
2013-12-07 04:41 Echelon9 Resolution fixed => reopened
2013-12-07 05:18 Echelon9 Changeset attached => fs2open trunk r10204
2013-12-07 05:18 Echelon9 Note Added: 0015510
2013-12-07 05:18 Echelon9 Status feedback => assigned
2013-12-07 05:18 Echelon9 Status assigned => resolved