Possible buffer over-read

[bnnm]

While doing some simple memtests (https://drmemory.org/) I get errors that I suspect mean BitReaderCxt in Atrac9Decode is reading over data buffer and not clamping to superframeSize. Doesn't seem to affect decoding though.

If I alloc a buffer with exactly superframeSize I multiple errors like this:

buffer = calloc(sizeof(uint8_t), info.superframeSize);
...
status = Atrac9Decode(handle, buffer, sample_buffer, &bytes_used);

Error #1: UNADDRESSABLE ACCESS beyond heap bounds: reading 0x038c70b1-0x038c70b2 1 byte(s)
# 0 libatrac9.dll!?                    +0x0      (0x6de81ea6 <libatrac9.dll+0x1ea6>)
# 1 libatrac9.dll!Atrac9GetCodecInfo   +0x141d   (0x6de8471e <libatrac9.dll+0x471e>)
# 2 libatrac9.dll!Atrac9GetCodecInfo   +0xff2    (0x6de842f3 <libatrac9.dll+0x42f3>)
# 3 libatrac9.dll!Atrac9GetCodecInfo   +0x8c7    (0x6de83bc8 <libatrac9.dll+0x3bc8>)
# 4 libatrac9.dll!?                    +0x0      (0x6de827ab <libatrac9.dll+0x27ab>)
# 5 libatrac9.dll!Atrac9Decode         +0x29     (0x6de8324a <libatrac9.dll+0x324a>)
# 6 decode_atrac9                       [coding/atrac9_decoder.c:118]
# 7 decode_vgmstream                    [C:\Users\bnnm\git\vgmstream\src/vgmstream.c:1977]
# 8 render_vgmstream_flat               [layout/flat.c:34]
# 9 render_vgmstream                    [C:\Users\bnnm\git\vgmstream\src/vgmstream.c:1040]
#10 main                                [C:\Users\bnnm\git\vgmstream\cli/vgmstream_cli.c:543]
Note: @0:00:01.713 in thread 7052
Note: next higher malloc: 0x038c70d8-0x038c80d8
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc:  0x038c6eb0-0x038c70b1
Note: instruction: movzx  0x02(%edi,%ecx) -> %eax

Which apparently refer to reads outside alloc'ed buffers: https://drmemory.org/docs/page_unaddr.html

If I instead alloc with some leeway I get no errors.

buffer = calloc(sizeof(uint8_t), info.superframeSize + 0x2); // +0x1 still gives some errors

It doesn't seem related to sample_buffer or other lib calls (if I mod buffer/remove calls I still get those errors).

[Thealexbarney]

That would be caused by the bit reader

LibAtrac9/C/src/bit_reader.c

Lines 34 to 68 in 6a9e00f

	int PeekInt(BitReaderCxt* br, const int bits)
	{
	const int byteIndex = br->Position / 8;
	const int bitIndex = br->Position % 8;
	const unsigned char* buffer = br->Buffer;
	if (bits <= 9)
	{
	int value = buffer[byteIndex] << 8 | buffer[byteIndex + 1];
	value &= 0xFFFF >> bitIndex;
	value >>= 16 - bits - bitIndex;
	return value;
	}
	if (bits <= 17)
	{
	int value = buffer[byteIndex] << 16 | buffer[byteIndex + 1] << 8 | buffer[byteIndex + 2];
	value &= 0xFFFFFF >> bitIndex;
	value >>= 24 - bits - bitIndex;
	return value;
	}
	if (bits <= 25)
	{
	int value = buffer[byteIndex] << 24
	| buffer[byteIndex + 1] << 16
	| buffer[byteIndex + 2] << 8
	| buffer[byteIndex + 3];
	value &= (int)(0xFFFFFFFF >> bitIndex);
	value >>= 32 - bits - bitIndex;
	return value;
	}
	return PeekIntFallback(br, bits);
	}

Depending on the size read, the current code may end up accessing one more byte than necessary. The official code does the same thing, but it always uses a buffer that's the size of the largest possible superframe, or 2048 bytes.