Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#25 lgeneral: Patch for lgeneral 1.2.3 due to bug write to unalloc'ed mem

open
nobody
None
5
2014-02-08
2014-02-08
Galland
No

I have found a bug in lgeneral v1.2.3, arising from writing beyond the end of an array when moving an unit. I have been able to get a savegame to reproduce the bug.
If you would load that game and find an infantry unit at position (32,5), and make it move to cell (25,7), the one isolated at the far left, the memory error will occur (some times the game crashes, or freezes, or it may continue with the memory error), with valgrind memchecker reporting an invalid write happening.
BUG SOURCE:
The bug is located at the loop in line 737 of "map.c", in the function "map_get_unit_way_points" because in this movement the unit uses its transport, which has a movement distance (unit->prop.mov) of 7, but the variable "reverse" was calloc'ed with a size unit->cur_mov, which is 4 (disembarked movement).
The error occurs because, on an embarked movement, *count goes above unit->cur_mov, hence producing a write to an unallocated position at "reverse[unit->cur_mov + 1]"
BUG SOLUTION:
Since the movement is legal (map_get_unit_move_mask is correct) then the problem is that the allocated array must be bigger to accommodate the largest movement allowed when using a transporter (which is larger than unit->cur_mov). My proposed fix is to substitute lines 732-733 of "map.c" with the following:
int maxpoints = unit->cur_mov;
if (mask[x][y].mount == 1) maxpoints = (unit->trsp_prop.mov>unit->prop.mov?unit->trsp_prop.mov:unit->prop.mov);
way = calloc( maxpoints + 1, sizeof( Way_Point ) );
reverse = calloc( maxpoints + 1, sizeof( Way_Point ) );
It won't hurt to allocate more size at times (like if there is not enough fuel) because this won't change the maximum allowed movement (the mask has not changed).
The solution is based on the "maxpoints" mask calculation in lines 610-624 of "map.c", function "map_get_unit_move_mask".
Attached is the patch, licensed as GPL.

1 Attachments

Discussion

  • Galland
    Galland
    2014-02-08

    If you load the attached savegame and find an infantry unit at position (32,5), and make it move to cell (25,7), the one isolated at the far left, the memory error will occur (some times the game crashes, or freezes, or it may continue with the memory error), with valgrind memchecker reporting an invalid write happening.

     
    Attachments