Impact
An unchecked offset and size used in a memcpy operation inside PCSX2’s CDVD SCMD 0x91 and SCMD 0x8F handler allows a specially crafted disc image or ELF to cause an out-of-bounds read from emulator memory. Because the offset and size is controlled through MG header fields, a specially crafted ELF can read data beyond the bounds of mg_buffer and have it reflected back into emulated memory.
Patches
Attached is a patch that should add sufficient bounds checking before copying memory.
diff --git a/pcsx2/CDVD/CDVD.cpp b/pcsx2/CDVD/CDVD.cpp
index 0c5ff56f02..8bece2d7e6 100644
--- a/pcsx2/CDVD/CDVD.cpp
+++ b/pcsx2/CDVD/CDVD.cpp
@@ -2942,8 +2942,19 @@ static void cdvdWrite16(u8 rt) // SCOMMAND
bit_ofs = mg_BIToffset(&cdvd.mg_buffer[0]);
- memcpy(&cdvd.mg_kbit[0], &cdvd.mg_buffer[bit_ofs - 0x20], 0x10);
- memcpy(&cdvd.mg_kcon[0], &cdvd.mg_buffer[bit_ofs - 0x10], 0x10);
+ const size_t buf_size = sizeof(cdvd.mg_buffer);
+
+ if (bit_ofs < 0x20 || (size_t)bit_ofs > buf_size)
+ {
+ fail_pol_cal();
+ break;
+ }
+
+ const size_t kbit_ofs = bit_ofs - 0x20;
+ const size_t kcon_ofs = bit_ofs - 0x10;
+
+ std::memcpy(&cdvd.mg_kbit[0], &cdvd.mg_buffer[kbit_ofs], 0x10);
+ std::memcpy(&cdvd.mg_kcon[0], &cdvd.mg_buffer[kcon_ofs], 0x10);
if ((cdvd.mg_buffer[bit_ofs + 5] || cdvd.mg_buffer[bit_ofs + 6] || cdvd.mg_buffer[bit_ofs + 7]) ||
(GetBufferU16(&cdvd.mg_buffer[0],bit_ofs + 4) * 16 + bit_ofs + 8 + 16 != GetBufferU16(&cdvd.mg_buffer[0], 0x14)))
@@ -2969,7 +2980,31 @@ static void cdvdWrite16(u8 rt) // SCOMMAND
{
SetSCMDResultSize(3); //in:0
const int bit_ofs = mg_BIToffset(&cdvd.mg_buffer[0]);
- memcpy(&cdvd.mg_buffer[0], &cdvd.mg_buffer[bit_ofs], static_cast<size_t>(8 + 16 * static_cast<int>(cdvd.mg_buffer[bit_ofs + 4])));
+
+ if (bit_ofs < 0)
+ {
+ fail_pol_cal();
+ break;
+ }
+
+ const size_t bufsize = sizeof(cdvd.mg_buffer);
+ const size_t ofs = static_cast<size_t>(bit_ofs);
+
+ if (ofs > bufsize - 5) // Make sure we can read the block count
+ {
+ fail_pol_cal();
+ break;
+ }
+ const unsigned int blocks = static_cast<unsigned int>(cdvd.mg_buffer[ofs + 4]);
+ const size_t copy_len = 8 + 16 * static_cast<size_t>(blocks);
+
+ if (copy_len > bufsize - ofs) // Make sure we can read the blocks
+ {
+ fail_pol_cal();
+ break;
+ }
+
+ std::memmove(&cdvd.mg_buffer[0], &cdvd.mg_buffer[ofs], copy_len);
cdvd.mg_maxsize = 0; // don't allow any write
cdvd.mg_size = 8 + 16 * cdvd.mg_buffer[4]; //new offset, i just moved the dataWorkarounds
Do not run software (homebrew) you do not trust on PCSX2.
References
#13693
slashbinslashcat Finder
F0bes Remediation developer
Impact
An unchecked offset and size used in a memcpy operation inside PCSX2’s CDVD SCMD 0x91 and SCMD 0x8F handler allows a specially crafted disc image or ELF to cause an out-of-bounds read from emulator memory. Because the offset and size is controlled through MG header fields, a specially crafted ELF can read data beyond the bounds of mg_buffer and have it reflected back into emulated memory.
Patches
Attached is a patch that should add sufficient bounds checking before copying memory.
Workarounds
Do not run software (homebrew) you do not trust on PCSX2.
References
#13693