mirror of
https://github.com/libretro/fuse-libretro.git
synced 2026-11-07 10:36:03 -10:00
libspectrum: don't prefetch past the end of the generalised data stream
get_generalised_data_bit() consumed a bit and then, if that exhausted the
current byte, immediately loaded the next one:
if( ++state->bits_through_byte == 8 ) {
state->bits_through_byte = 0;
state->bytes_through_stream++;
state->current_byte = block->data[ state->bytes_through_stream ];
}
The load is unconditional on the byte boundary, including the boundary at the
very end of the stream, so the final bit of the last byte fetches
data[ data_count ]. That is one byte past the allocation whenever the bit
stream is a whole number of bytes long, i.e. whenever
bits_per_data_symbol * TOTD is a multiple of 8.
This is not a malformed-input case. It fires on well-formed tapes, and a
byte-aligned data stream is the common case rather than the exception: a
block with a two-symbol data alphabet and any multiple of eight data symbols
hits it, as does any two- or four-bit alphabet with a multiple of four or two
symbols respectively. ASan reports a 1-byte heap-buffer-overflow read in
get_generalised_data_bit() on a 36-byte TZX with TOTD=8, NPD=1, ASD=2 and a
single data byte.
Move the fetch to the top of the function so a byte is only loaded when a bit
is actually wanted from it. The final byte's last bit no longer pulls in a
successor that does not exist, and bytes_through_stream stops one short of
data_count instead of reaching it.
The decoded bit sequence is unchanged: bits_through_byte simply holds 8
between the last bit of one byte and the first bit of the next, rather than
being reset early. Verified by diffing full ( tstates, flags ) edge traces
before and after across 1-, 2- and 3-bit symbol widths, byte-aligned and
non-byte-aligned streams, power-of-two and non-power-of-two alphabets, and
blocks with and without a pilot run: identical in every case.
This commit is contained in:
1 parent
6e96da12b0
commit
2a5f1d43fe
1 file changed
+11
-4
+11
-4
@@ -933,15 +933,22 @@ static libspectrum_byte
|
||||
get_generalised_data_bit( libspectrum_tape_generalised_data_block *block,
|
||||
libspectrum_tape_generalised_data_block_state *state )
|
||||
{
|
||||
libspectrum_byte r = state->current_byte & 0x80 ? 1 : 0;
|
||||
state->current_byte <<= 1;
|
||||
libspectrum_byte r;
|
||||
|
||||
if( ++state->bits_through_byte == 8 ) {
|
||||
/* Fetch the next byte when a bit is actually wanted from it, not as soon as
|
||||
the previous byte is exhausted: prefetching reads one byte past the data
|
||||
stream whenever the stream is a whole number of bytes long, which is the
|
||||
common case. */
|
||||
if( state->bits_through_byte == 8 ) {
|
||||
state->bits_through_byte = 0;
|
||||
state->bytes_through_stream++;
|
||||
state->current_byte = block->data[ state->bytes_through_stream ];
|
||||
}
|
||||
|
||||
|
||||
r = state->current_byte & 0x80 ? 1 : 0;
|
||||
state->current_byte <<= 1;
|
||||
state->bits_through_byte++;
|
||||
|
||||
return r;
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user