Improved sanitation for save states for better security and stability

This commit is contained in:
Lior Halphon 2021-10-30 16:03:13 +03:00
parent 8d319c65c2
commit 4498d16bed
9 changed files with 73 additions and 15 deletions

View File

@ -658,7 +658,7 @@ static const uint8_t workboy_vk_to_key[] = {
if ( [[pboard types] containsObject:NSURLPboardType] ) { if ( [[pboard types] containsObject:NSURLPboardType] ) {
NSURL *fileURL = [NSURL URLFromPasteboard:pboard]; NSURL *fileURL = [NSURL URLFromPasteboard:pboard];
if (GB_is_stave_state(fileURL.fileSystemRepresentation)) { if (GB_is_save_state(fileURL.fileSystemRepresentation)) {
return NSDragOperationGeneric; return NSDragOperationGeneric;
} }
} }

View File

@ -24,6 +24,7 @@
#include "cheats.h" #include "cheats.h"
#include "rumble.h" #include "rumble.h"
#include "workboy.h" #include "workboy.h"
#include "random.h"
#define GB_STRUCT_VERSION 13 #define GB_STRUCT_VERSION 13
@ -554,7 +555,7 @@ struct GB_gameboy_internal_s {
/* Video Display */ /* Video Display */
GB_SECTION(video, GB_SECTION(video,
uint32_t vram_size; // Different between CGB and DMG uint32_t vram_size; // Different between CGB and DMG
uint8_t cgb_vram_bank; bool cgb_vram_bank;
uint8_t oam[0xA0]; uint8_t oam[0xA0];
uint8_t background_palettes_data[0x40]; uint8_t background_palettes_data[0x40];
uint8_t sprite_palettes_data[0x40]; uint8_t sprite_palettes_data[0x40];

View File

@ -52,6 +52,7 @@ void GB_update_joyp(GB_gameboy_t *gb)
break; break;
default: default:
__builtin_unreachable();
break; break;
} }

View File

@ -291,7 +291,7 @@ static uint8_t read_vram(GB_gameboy_t *gb, uint16_t addr)
addr = gb->last_tile_data_address; addr = gb->last_tile_data_address;
} }
} }
return gb->vram[(addr & 0x1FFF) + (uint16_t) gb->cgb_vram_bank * 0x2000]; return gb->vram[(addr & 0x1FFF) + (gb->cgb_vram_bank? 0x2000 : 0)];
} }
static uint8_t read_mbc_ram(GB_gameboy_t *gb, uint16_t addr) static uint8_t read_mbc_ram(GB_gameboy_t *gb, uint16_t addr)
@ -464,7 +464,7 @@ static uint8_t read_high_memory(GB_gameboy_t *gb, uint16_t addr)
break; break;
default: default:
break; __builtin_unreachable();
} }
for (unsigned i = 0; i < 8; i++) { for (unsigned i = 0; i < 8; i++) {
@ -635,8 +635,7 @@ static uint8_t read_high_memory(GB_gameboy_t *gb, uint16_t addr)
} }
return 0xFF; return 0xFF;
} }
/* Hardware registers */ __builtin_unreachable();
return 0;
} }
if (addr == 0xFFFF) { if (addr == 0xFFFF) {
@ -815,7 +814,7 @@ static void write_vram(GB_gameboy_t *gb, uint16_t addr, uint8_t value)
addr = gb->last_tile_data_address; addr = gb->last_tile_data_address;
} }
} }
gb->vram[(addr & 0x1FFF) + (uint16_t) gb->cgb_vram_bank * 0x2000] = value; gb->vram[(addr & 0x1FFF) + (gb->cgb_vram_bank? 0x2000 : 0)] = value;
} }
static bool huc3_write(GB_gameboy_t *gb, uint8_t value) static bool huc3_write(GB_gameboy_t *gb, uint8_t value)

View File

@ -307,6 +307,11 @@ static bool verify_and_update_state_compatibility(GB_gameboy_t *gb, GB_gameboy_t
return false; return false;
} }
if (GB_is_cgb(gb) != GB_is_cgb(save) || GB_is_hle_sgb(gb) != GB_is_hle_sgb(save)) {
GB_log(gb, "The save state is for a different Game Boy model. Try changing the emulated model.\n");
return false;
}
if (gb->mbc_ram_size < save->mbc_ram_size) { if (gb->mbc_ram_size < save->mbc_ram_size) {
GB_log(gb, "The save state has non-matching MBC RAM size.\n"); GB_log(gb, "The save state has non-matching MBC RAM size.\n");
return false; return false;
@ -333,8 +338,27 @@ static bool verify_and_update_state_compatibility(GB_gameboy_t *gb, GB_gameboy_t
} }
} }
switch (save->model) {
case GB_MODEL_DMG_B: return true;
case GB_MODEL_SGB_NTSC: return true;
case GB_MODEL_SGB_PAL: return true;
case GB_MODEL_SGB_NTSC_NO_SFC: return true;
case GB_MODEL_SGB_PAL_NO_SFC: return true;
case GB_MODEL_MGB: return true;
case GB_MODEL_SGB2: return true;
case GB_MODEL_SGB2_NO_SFC: return true;
case GB_MODEL_CGB_C: return true;
case GB_MODEL_CGB_D: return true;
case GB_MODEL_CGB_E: return true;
case GB_MODEL_AGB: return true;
}
if ((gb->model & GB_MODEL_FAMILY_MASK) == (save->model & GB_MODEL_FAMILY_MASK)) {
save->model = gb->model;
return true; return true;
} }
GB_log(gb, "This save state is for an unknown Game Boy model\n");
return false;
}
static void sanitize_state(GB_gameboy_t *gb) static void sanitize_state(GB_gameboy_t *gb)
{ {
@ -347,6 +371,35 @@ static void sanitize_state(GB_gameboy_t *gb)
gb->bg_fifo.write_end &= 0xF; gb->bg_fifo.write_end &= 0xF;
gb->oam_fifo.read_end &= 0xF; gb->oam_fifo.read_end &= 0xF;
gb->oam_fifo.write_end &= 0xF; gb->oam_fifo.write_end &= 0xF;
gb->last_tile_index_address &= 0x1FFF;
gb->window_tile_x &= 0x1F;
/* These are kind of DOS-ish if too large */
if (abs(gb->display_cycles) > 0x8000) {
gb->display_cycles = 0;
}
if (abs(gb->div_cycles) > 0x8000) {
gb->div_cycles = 0;
}
if (!GB_is_cgb(gb)) {
gb->cgb_mode = false;
}
if (gb->ram_size == 0x8000) {
gb->cgb_ram_bank &= 0x7;
}
else {
gb->cgb_ram_bank = 1;
}
if (gb->vram_size != 0x4000) {
gb->cgb_vram_bank = 0;
}
if (!GB_is_cgb(gb)) {
gb->current_tile_attributes = 0;
}
gb->object_low_line_address &= gb->vram_size & ~1; gb->object_low_line_address &= gb->vram_size & ~1;
if (gb->lcd_x > gb->position_in_line) { if (gb->lcd_x > gb->position_in_line) {
gb->lcd_x = gb->position_in_line; gb->lcd_x = gb->position_in_line;
@ -919,6 +972,8 @@ static int load_bess_save(GB_gameboy_t *gb, virtual_file_t *file, bool is_samebo
save.halted = core.execution_mode == 1; save.halted = core.execution_mode == 1;
save.stopped = core.execution_mode == 2; save.stopped = core.execution_mode == 2;
// Done early for compatibility with 0.14.x
GB_write_memory(&save, 0xFF00 + GB_IO_SVBK, core.io_registers[GB_IO_SVBK]);
// CPU related // CPU related
// Determines DMG mode // Determines DMG mode
@ -979,7 +1034,6 @@ static int load_bess_save(GB_gameboy_t *gb, virtual_file_t *file, bool is_samebo
GB_write_memory(&save, 0xFF00 + GB_IO_BGPI, core.io_registers[GB_IO_BGPI]); GB_write_memory(&save, 0xFF00 + GB_IO_BGPI, core.io_registers[GB_IO_BGPI]);
GB_write_memory(&save, 0xFF00 + GB_IO_OBPI, core.io_registers[GB_IO_OBPI]); GB_write_memory(&save, 0xFF00 + GB_IO_OBPI, core.io_registers[GB_IO_OBPI]);
GB_write_memory(&save, 0xFF00 + GB_IO_OPRI, core.io_registers[GB_IO_OPRI]); GB_write_memory(&save, 0xFF00 + GB_IO_OPRI, core.io_registers[GB_IO_OPRI]);
GB_write_memory(&save, 0xFF00 + GB_IO_SVBK, core.io_registers[GB_IO_SVBK]);
// Interrupts // Interrupts
GB_write_memory(&save, 0xFF00 + GB_IO_IF, core.io_registers[GB_IO_IF]); GB_write_memory(&save, 0xFF00 + GB_IO_IF, core.io_registers[GB_IO_IF]);
@ -1018,6 +1072,7 @@ static int load_bess_save(GB_gameboy_t *gb, virtual_file_t *file, bool is_samebo
case BE32('MBC '): case BE32('MBC '):
if (!found_core) goto parse_error; if (!found_core) goto parse_error;
if (LE32(block.size) % 3 != 0) goto parse_error; if (LE32(block.size) % 3 != 0) goto parse_error;
if (LE32(block.size) > 0x1000) goto parse_error;
for (unsigned i = LE32(block.size); i > 0; i -= 3) { for (unsigned i = LE32(block.size); i > 0; i -= 3) {
BESS_MBC_pair_t pair; BESS_MBC_pair_t pair;
file->read(file, &pair, sizeof(pair)); file->read(file, &pair, sizeof(pair));
@ -1165,6 +1220,7 @@ error:
GB_log(gb, "Attempted to import a save state from a different emulator or incompatible version, but the save state is invalid.\n"); GB_log(gb, "Attempted to import a save state from a different emulator or incompatible version, but the save state is invalid.\n");
} }
GB_free(&save); GB_free(&save);
sanitize_state(gb);
return errno; return errno;
} }
@ -1272,7 +1328,7 @@ int GB_load_state_from_buffer(GB_gameboy_t *gb, const uint8_t *buffer, size_t le
} }
bool GB_is_stave_state(const char *path) bool GB_is_save_state(const char *path)
{ {
bool ret = false; bool ret = false;
FILE *f = fopen(path, "rb"); FILE *f = fopen(path, "rb");

View File

@ -27,7 +27,7 @@ void GB_save_state_to_buffer(GB_gameboy_t *gb, uint8_t *buffer);
int GB_load_state(GB_gameboy_t *gb, const char *path); int GB_load_state(GB_gameboy_t *gb, const char *path);
int GB_load_state_from_buffer(GB_gameboy_t *gb, const uint8_t *buffer, size_t length); int GB_load_state_from_buffer(GB_gameboy_t *gb, const uint8_t *buffer, size_t length);
bool GB_is_stave_state(const char *path); bool GB_is_save_state(const char *path);
#ifdef GB_INTERNAL #ifdef GB_INTERNAL
static inline uint32_t state_magic(void) static inline uint32_t state_magic(void)
{ {

View File

@ -656,6 +656,7 @@ static bool condition_code(GB_gameboy_t *gb, uint8_t opcode)
case 3: case 3:
return (gb->af & GB_CARRY_FLAG); return (gb->af & GB_CARRY_FLAG);
} }
__builtin_unreachable();
return false; return false;
} }

View File

@ -1344,7 +1344,7 @@ void run_gui(bool is_running)
break; break;
} }
case SDL_DROPFILE: { case SDL_DROPFILE: {
if (GB_is_stave_state(event.drop.file)) { if (GB_is_save_state(event.drop.file)) {
if (GB_is_inited(&gb)) { if (GB_is_inited(&gb)) {
dropped_state_file = event.drop.file; dropped_state_file = event.drop.file;
pending_command = GB_SDL_LOAD_STATE_FROM_FILE_COMMAND; pending_command = GB_SDL_LOAD_STATE_FROM_FILE_COMMAND;

View File

@ -225,7 +225,7 @@ static void handle_events(GB_gameboy_t *gb)
break; break;
case SDL_DROPFILE: { case SDL_DROPFILE: {
if (GB_is_stave_state(event.drop.file)) { if (GB_is_save_state(event.drop.file)) {
dropped_state_file = event.drop.file; dropped_state_file = event.drop.file;
pending_command = GB_SDL_LOAD_STATE_FROM_FILE_COMMAND; pending_command = GB_SDL_LOAD_STATE_FROM_FILE_COMMAND;
} }