ISOFileSystem: Don't copy out uninitialized stack on a failed block read

ReadFile and the read-path-table ioctl both read a sector into a stack buffer
and memcpy it to the destination without checking whether the read succeeded.
FileBlockDevice::ReadBlock returns false on a short read and leaves the buffer
untouched, so a read past the end of a truncated or crafted image copies 2KB of
uninitialized host stack into guest-visible memory.

Zero the buffer on failure, and bail out of the ioctl if the volume descriptor
can't be read instead of using a garbage path table length. The constructor
already checked that same read.

ReadBlocks writes straight into the caller's buffer, so a partial read there
leaves stale data rather than host memory - left alone deliberately, since
zeroing it would throw away the valid prefix on a truncated image.
This commit is contained in:
Henrik Rydgård
2026-09-04 13:02:57 -06:00
parent 9c1f55ed76
commit 2994145f86
+16 -4
View File
@@ -499,7 +499,11 @@ int ISOFileSystem::Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outd
}
VolDescriptor desc;
blockDevice->ReadBlock(16, (u8 *)&desc);
if (!blockDevice->ReadBlock(16, (u8 *)&desc)) {
blockDevice->NotifyReadError();
ERROR_LOG(Log::FileSystem, "Failed to read volume descriptor for the path table");
return SCE_KERNEL_ERROR_ERRNO_IO_ERROR;
}
if (outlen < (u32)desc.pathTableLength) {
return SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT;
} else {
@@ -517,7 +521,9 @@ int ISOFileSystem::Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outd
u8 temp[2048];
// `blocks` whole sectors starting at `block` were already consumed by
// ReadBlocks() above, so the trailing partial sector is the next one.
blockDevice->ReadBlock(block + blocks, temp);
if (!blockDevice->ReadBlock(block + blocks, temp)) {
memset(temp, 0, sizeof(temp));
}
memcpy(out, temp, size);
}
return 0;
@@ -616,7 +622,11 @@ size_t ISOFileSystem::ReadFile(u32 handle, u8 *pointer, s64 size, int &usec) {
const u8 *const start = pointer;
if (firstBlockSize > 0) {
blockDevice->ReadBlock(secNum++, theSector);
// theSector is uninitialized stack memory, so on a failed read we must not copy it out -
// that would hand host stack contents to the game.
if (!blockDevice->ReadBlock(secNum++, theSector)) {
memset(theSector, 0, sizeof(theSector));
}
memcpy(pointer, theSector + firstBlockOffset, firstBlockSize);
pointer += firstBlockSize;
}
@@ -627,7 +637,9 @@ size_t ISOFileSystem::ReadFile(u32 handle, u8 *pointer, s64 size, int &usec) {
pointer += middleSize;
}
if (lastBlockSize > 0) {
blockDevice->ReadBlock(secNum++, theSector);
if (!blockDevice->ReadBlock(secNum++, theSector)) {
memset(theSector, 0, sizeof(theSector));
}
memcpy(pointer, theSector, lastBlockSize);
pointer += lastBlockSize;
}