We already had code to close open host files when reading or writing a
savestate, but due to d35fe1b we also need to close open guest files
when reading a savestate, otherwise DoStateRead fails to delete them.
I was considering an alternative solution where instead of copying and
clearing m_handles, we just set `handle.opened = false;` for each handle
before reading a savestate (but not before writing a savestate).
However, this wouldn't solve the problem of DoStateWriteOrMeasure's
calls to OpenFile failing due to all handles being open. I'm not aware
of any games that have that many handles open, though.
Log each rejected ELF header, range, and symbol reference. This
provides actionable diagnostics for malformed files without changing the
validation behavior.
ElfReader trusted table offsets and counts from standalone ELF files.
Malformed input could make it read and write past the loaded file buffer.
Validate the ELF header, table ranges, segment data, section data, and
string-table references before accessing them. Invalid files use the
existing executable boot failure path.
NetPlay save synchronization decoded remote LZO blocks with the unsafe
decoder and no output capacity. A malicious host could overflow a client
buffer with a block larger than its declared size.
Use the bounds-checking decoder, validate the declared output length, and
grow buffer results only after each checked block has been decoded.
A malicious dol could theoretically use integer wraparound to bypass
bounds checking and cause DolReader to read past the end of m_bytes.
Could result in crashes, wasting large amounts of memory, or even the
disclosure of heap memory contents.
A malicious GCZ file could probably force this to be negative.
Shouldn't cause any issues other than file read failures, but need to fix
because it is causing errors on MSVC.
SectorReader::ReadChunk does do some validation on it, but it only
checks against the original disc size (reported by the GCZ file).
It has no idea how many blocks the header claimed the disc had.
A maliciously crafted GCZ file could trigger read overflows off the end
of the m_block_pointers/m_hashes arrays.
Independently spotted by both @Dentomologist and me while reviewing
PR #14805
The previous limit was correct for valid VPBs, but an invalid VPB
controlled by a malicious game could contain a non-fractional value
in current_pos_frac, which would allow writing an extra 15 samples
(30 bytes) into the stack. With AFC encoding, this is rounded up to 16
samples, but with much less control over which bytes are written.
Maybe we should be doing some validation, or bounds checking, but I'm
pretty sure this issue was copied from the original ucode, and we kinda
want to stay compatible.
The simpler fix is to just increase the size of raw_input_samples.
I've checked other code paths, and 0x514 samples seems to be the limit.
// Maximum of 0x500 samples here - see NeededRawSamplesCount to understand
// this practical limit (resampling_ratio = 0xFFFF -> 0x500 samples). Add a
// margin of 4 that is needed for samples source that do resampling.
std::array<s16,0x500+4>raw_input_samples;
// this practical limit (resampling_ratio = 0xFFFF -> 0x500 samples).
//
// If current_pos_frac contains an (invalid) non-fractional part, it can push
// this up by another 15 samples. Which DownloadAFCSamplesFromARAM then rounds
// up to the next multiple of 16. So add an extra 0x10 samples to be safe.
//
// Plus we need an extra four samples at the start to hold the last four
// samples from the previous frame.
std::array<s16,4+0x500+0x10>raw_input_samples;
for(size_ti=0;i<4;++i)
raw_input_samples[i]=vpb->resample_buffer[i];
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.