EXL / NXEngine

A port of the open-source rewrite Cave Story game engine for various platforms. Original author is Caitlin "rogueeve" Shaw, https://nxengine.sourceforge.io/
https://exlmoto.ru/nxengine/
GNU General Public License v3.0
170 stars 35 forks source link

fix crash in profile_load #9

Closed FridayOrtiz closed 2 years ago

FridayOrtiz commented 2 years ago

There are two crashes/segfaults in profile_load which could be turned into arbitrary writes. First, the fgetl and fgeti functions don't check if you've reached the end of the file, which causes a too-small profile.dat to fill the Profile with whatever garbage stack values and leads to a crash, if you call profile_load directly.

00000000: 446f 3034 3132 3230                      Do041220

In the next crash we set the item type to some large value, like 0x5C000000.

00000000: 446f 3034 3132 3230 0d00 0000 0800 0000  Do041220........
00000010: 2de6 0100 20e0 0000 0200 0000 0300 0000  -... ...........
00000020: 0300 0000 0000 0000 0000 0000 0000 0000  ................
00000030: 0000 0000 0000 0000 0000 005c 5c5c 5c5c  ...........\\\\\
00000040: 5c5c 5c5c 5c5c 5c5c 5c5c 5c5c            \\\\\\\\\\\\

When we reach the following code block, we'll attempt to write to some large offset and most likely segfault.

        int level = fgetl(fp);
        int xp = fgetl(fp);
        int maxammo = fgetl(fp);
        int ammo = fgetl(fp);

        file->weapons[type].hasWeapon = true;
        file->weapons[type].level = (level - 1);
        file->weapons[type].xp = xp;
        file->weapons[type].ammo = ammo;
        file->weapons[type].maxammo = maxammo;

This patch fixes the two crashes by adding a check to the fgeti and fgetl functions and making sure the type is within the maximum number of weapons.

Note: I have no idea if this will break existing saves in some way, but I don't think it will.

EXL commented 2 years ago

@FridayOrtiz Thank you!