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.
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.
glMemoryBarrier comes from ARB_shader_image_load_store, so on a desktop context
below GL 4.2 the pointer is null. bSupportsBBox is set from
bSupportsFragmentStoresAndAtomics, which such a driver can still have, so saving
a state reads the bounding box and calls a null pointer.
UsePersistentStagingBuffers() in OGLTexture.cpp already guards the same call with
bSupportsImageLoadStore.
This started as a fixing a misalignment issue, but I got carried away.
Was originally 41 bytes (overflowing it's expected 40 bytes by 1 bit).
Now it's 27 bytes plus 8 bits of padding (ready for future expansion).
The savings come from:
- Removing UV usage from components, it can be reconstructed from
texcoord_elem_count (saved 8 bits)
- Removed the 8 unused bits from texMtxInfo_n_projection (saved 8 bits)
- Removing unused bit from from start of components (saved 1 bit)
- Removed extra bit from inputform, texgentype and sourcerow
- packed texMtxInfo_n_projection and texcoord_elem_count back into
the per texgen info space freed up above. (saved 24 bits)
- Overlayed postMtx index and Emboss mode shifts into a union based on
texgentype (saved 56 bits)
- Move to a single-bit union tag, freeing up an extra bit for regular
texgens.
- Move PotMtx normalize into the freed up space (saved 8 bits)
Total savings: 105 bits of data
EDIT: Even worse, MSVC wasn't respecting pack(1) for bitfields at all,
so on windows this struct was actually quite a bit larger.
Something like 80 bytes, if not more.
Fixed this by dropping the size of any enums used in these
bit structs to u8. Our Common::BitField allows for the underlying
type to be explicitly specified to something larger.
msvc's bitfield packing appears to be completely braindead, can't
mix sizes at all.
msg_in and msg_out were ending up with only 4-byte alignment, which
is undefined behaviour, since they contain 64-bit fields. Probably
doesn't cause any issues, but it showed up in ubsan.
Scott Mansell