From 4498d16bed3f98fd0dc0654805d0fa6e50007e63 Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Sat, 30 Oct 2021 16:03:13 +0300 Subject: [PATCH] Improved sanitation for save states for better security and stability --- Cocoa/GBView.m | 2 +- Core/gb.h | 3 ++- Core/joypad.c | 1 + Core/memory.c | 11 ++++---- Core/save_state.c | 64 ++++++++++++++++++++++++++++++++++++++++++++--- Core/save_state.h | 2 +- Core/sm83_cpu.c | 1 + SDL/gui.c | 2 +- SDL/main.c | 2 +- 9 files changed, 73 insertions(+), 15 deletions(-) diff --git a/Cocoa/GBView.m b/Cocoa/GBView.m index 39a22e8..6c92c3f 100644 --- a/Cocoa/GBView.m +++ b/Cocoa/GBView.m @@ -658,7 +658,7 @@ static const uint8_t workboy_vk_to_key[] = { if ( [[pboard types] containsObject:NSURLPboardType] ) { NSURL *fileURL = [NSURL URLFromPasteboard:pboard]; - if (GB_is_stave_state(fileURL.fileSystemRepresentation)) { + if (GB_is_save_state(fileURL.fileSystemRepresentation)) { return NSDragOperationGeneric; } } diff --git a/Core/gb.h b/Core/gb.h index ea4fe7d..655346b 100644 --- a/Core/gb.h +++ b/Core/gb.h @@ -24,6 +24,7 @@ #include "cheats.h" #include "rumble.h" #include "workboy.h" +#include "random.h" #define GB_STRUCT_VERSION 13 @@ -554,7 +555,7 @@ struct GB_gameboy_internal_s { /* Video Display */ GB_SECTION(video, uint32_t vram_size; // Different between CGB and DMG - uint8_t cgb_vram_bank; + bool cgb_vram_bank; uint8_t oam[0xA0]; uint8_t background_palettes_data[0x40]; uint8_t sprite_palettes_data[0x40]; diff --git a/Core/joypad.c b/Core/joypad.c index b30258c..c0655f0 100644 --- a/Core/joypad.c +++ b/Core/joypad.c @@ -52,6 +52,7 @@ void GB_update_joyp(GB_gameboy_t *gb) break; default: + __builtin_unreachable(); break; } diff --git a/Core/memory.c b/Core/memory.c index 2fdc72d..426e5d6 100644 --- a/Core/memory.c +++ b/Core/memory.c @@ -291,7 +291,7 @@ static uint8_t read_vram(GB_gameboy_t *gb, uint16_t addr) 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) @@ -464,7 +464,7 @@ static uint8_t read_high_memory(GB_gameboy_t *gb, uint16_t addr) break; default: - break; + __builtin_unreachable(); } 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; } - /* Hardware registers */ - return 0; + __builtin_unreachable(); } 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; } } - 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) @@ -1395,7 +1394,7 @@ static GB_write_function_t * const write_map[] = write_vram, write_vram, /* 8XXX, 9XXX */ write_mbc_ram, write_mbc_ram, /* AXXX, BXXX */ write_ram, write_banked_ram, /* CXXX, DXXX */ - write_ram, write_high_memory, /* EXXX FXXX */ + write_ram, write_high_memory, /* EXXX FXXX */ }; void GB_write_memory(GB_gameboy_t *gb, uint16_t addr, uint8_t value) diff --git a/Core/save_state.c b/Core/save_state.c index 6e7ca9d..715baa9 100644 --- a/Core/save_state.c +++ b/Core/save_state.c @@ -307,6 +307,11 @@ static bool verify_and_update_state_compatibility(GB_gameboy_t *gb, GB_gameboy_t 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) { GB_log(gb, "The save state has non-matching MBC RAM size.\n"); return false; @@ -333,7 +338,26 @@ static bool verify_and_update_state_compatibility(GB_gameboy_t *gb, GB_gameboy_t } } - return true; + 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; + } + GB_log(gb, "This save state is for an unknown Game Boy model\n"); + return false; } 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->oam_fifo.read_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; if (gb->lcd_x > gb->position_in_line) { gb->lcd_x = gb->position_in_line; @@ -918,7 +971,9 @@ static int load_bess_save(GB_gameboy_t *gb, virtual_file_t *file, bool is_samebo save.halted = core.execution_mode == 1; 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 // 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_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_SVBK, core.io_registers[GB_IO_SVBK]); // Interrupts 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 '): if (!found_core) 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) { BESS_MBC_pair_t 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_free(&save); + sanitize_state(gb); 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; FILE *f = fopen(path, "rb"); diff --git a/Core/save_state.h b/Core/save_state.h index 4572a72..bf43a65 100644 --- a/Core/save_state.h +++ b/Core/save_state.h @@ -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_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 static inline uint32_t state_magic(void) { diff --git a/Core/sm83_cpu.c b/Core/sm83_cpu.c index a918081..d4829d5 100644 --- a/Core/sm83_cpu.c +++ b/Core/sm83_cpu.c @@ -656,6 +656,7 @@ static bool condition_code(GB_gameboy_t *gb, uint8_t opcode) case 3: return (gb->af & GB_CARRY_FLAG); } + __builtin_unreachable(); return false; } diff --git a/SDL/gui.c b/SDL/gui.c index 6bccd87..d9a58e5 100644 --- a/SDL/gui.c +++ b/SDL/gui.c @@ -1344,7 +1344,7 @@ void run_gui(bool is_running) break; } case SDL_DROPFILE: { - if (GB_is_stave_state(event.drop.file)) { + if (GB_is_save_state(event.drop.file)) { if (GB_is_inited(&gb)) { dropped_state_file = event.drop.file; pending_command = GB_SDL_LOAD_STATE_FROM_FILE_COMMAND; diff --git a/SDL/main.c b/SDL/main.c index c0fd186..ccfa906 100644 --- a/SDL/main.c +++ b/SDL/main.c @@ -225,7 +225,7 @@ static void handle_events(GB_gameboy_t *gb) break; 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; pending_command = GB_SDL_LOAD_STATE_FROM_FILE_COMMAND; }