From 60e16c15b6e9351371711356d205a12435b3c574 Mon Sep 17 00:00:00 2001 From: gdkchan Date: Tue, 4 Oct 2022 20:12:54 -0300 Subject: [PATCH] Fix memory corruption in BCAT and FS Read methods when buffer is larger than needed (#3739) * Fix memory corruption in FS Read methods when buffer is larger than needed * PR feedback * nit: Don't move this around --- .../IDeliveryCacheDirectoryService.cs | 17 ++++++++--------- .../IDeliveryCacheFileService.cs | 17 ++++++++--------- .../IDeliveryCacheStorageService.cs | 17 ++++++++--------- .../Services/Fs/FileSystemProxy/IDirectory.cs | 14 +++++++------- .../HOS/Services/Fs/FileSystemProxy/IFile.cs | 16 ++++++++-------- .../Services/Fs/FileSystemProxy/IFileSystem.cs | 8 +------- .../Services/Fs/FileSystemProxy/IStorage.cs | 18 +++++++++--------- .../HOS/Services/Fs/IFileSystemProxy.cs | 14 +++++++------- .../HOS/Services/Fs/ISaveDataInfoReader.cs | 14 +++++++------- .../Services/Ssl/SslService/ISslConnection.cs | 13 ++++++------- 10 files changed, 69 insertions(+), 79 deletions(-) diff --git a/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheDirectoryService.cs b/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheDirectoryService.cs index 34176b4e3..e7ccb6d09 100644 --- a/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheDirectoryService.cs +++ b/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheDirectoryService.cs @@ -38,18 +38,17 @@ namespace Ryujinx.HLE.HOS.Services.Bcat.ServiceCreator // Read() -> (u32, buffer) public ResultCode Read(ServiceCtx context) { - ulong position = context.Request.ReceiveBuff[0].Position; - ulong size = context.Request.ReceiveBuff[0].Size; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; + ulong bufferLen = context.Request.ReceiveBuff[0].Size; - byte[] data = new byte[size]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _base.Get.Read(out int entriesRead, MemoryMarshal.Cast(region.Memory.Span)); - Result result = _base.Get.Read(out int entriesRead, MemoryMarshal.Cast(data)); + context.ResponseData.Write(entriesRead); - context.Memory.Write(position, data); - - context.ResponseData.Write(entriesRead); - - return (ResultCode)result.Value; + return (ResultCode)result.Value; + } } [CommandHipc(2)] diff --git a/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheFileService.cs b/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheFileService.cs index b7b034476..c7706f921 100644 --- a/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheFileService.cs +++ b/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheFileService.cs @@ -38,20 +38,19 @@ namespace Ryujinx.HLE.HOS.Services.Bcat.ServiceCreator // Read(u64) -> (u64, buffer) public ResultCode Read(ServiceCtx context) { - ulong position = context.Request.ReceiveBuff[0].Position; - ulong size = context.Request.ReceiveBuff[0].Size; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; + ulong bufferLen = context.Request.ReceiveBuff[0].Size; long offset = context.RequestData.ReadInt64(); - byte[] data = new byte[size]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _base.Get.Read(out long bytesRead, offset, region.Memory.Span); - Result result = _base.Get.Read(out long bytesRead, offset, data); + context.ResponseData.Write(bytesRead); - context.Memory.Write(position, data); - - context.ResponseData.Write(bytesRead); - - return (ResultCode)result.Value; + return (ResultCode)result.Value; + } } [CommandHipc(2)] diff --git a/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheStorageService.cs b/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheStorageService.cs index edb4b03aa..71d7aed70 100644 --- a/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheStorageService.cs +++ b/Ryujinx.HLE/HOS/Services/Bcat/ServiceCreator/IDeliveryCacheStorageService.cs @@ -50,18 +50,17 @@ namespace Ryujinx.HLE.HOS.Services.Bcat.ServiceCreator // EnumerateDeliveryCacheDirectory() -> (u32, buffer) public ResultCode EnumerateDeliveryCacheDirectory(ServiceCtx context) { - ulong position = context.Request.ReceiveBuff[0].Position; - ulong size = context.Request.ReceiveBuff[0].Size; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; + ulong bufferLen = context.Request.ReceiveBuff[0].Size; - byte[] data = new byte[size]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _base.Get.EnumerateDeliveryCacheDirectory(out int count, MemoryMarshal.Cast(region.Memory.Span)); - Result result = _base.Get.EnumerateDeliveryCacheDirectory(out int count, MemoryMarshal.Cast(data)); + context.ResponseData.Write(count); - context.Memory.Write(position, data); - - context.ResponseData.Write(count); - - return (ResultCode)result.Value; + return (ResultCode)result.Value; + } } protected override void Dispose(bool isDisposing) diff --git a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IDirectory.cs b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IDirectory.cs index b04e8675e..bfe13592a 100644 --- a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IDirectory.cs +++ b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IDirectory.cs @@ -17,17 +17,17 @@ namespace Ryujinx.HLE.HOS.Services.Fs.FileSystemProxy // Read() -> (u64 count, buffer entries) public ResultCode Read(ServiceCtx context) { - ulong bufferPosition = context.Request.ReceiveBuff[0].Position; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; ulong bufferLen = context.Request.ReceiveBuff[0].Size; - byte[] entryBuffer = new byte[bufferLen]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _baseDirectory.Get.Read(out long entriesRead, new OutBuffer(region.Memory.Span)); - Result result = _baseDirectory.Get.Read(out long entriesRead, new OutBuffer(entryBuffer)); + context.ResponseData.Write(entriesRead); - context.Memory.Write(bufferPosition, entryBuffer); - context.ResponseData.Write(entriesRead); - - return (ResultCode)result.Value; + return (ResultCode)result.Value; + } } [CommandHipc(1)] diff --git a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFile.cs b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFile.cs index fa5e05d65..878fcacf6 100644 --- a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFile.cs +++ b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFile.cs @@ -19,7 +19,8 @@ namespace Ryujinx.HLE.HOS.Services.Fs.FileSystemProxy // Read(u32 readOption, u64 offset, u64 size) -> (u64 out_size, buffer out_buf) public ResultCode Read(ServiceCtx context) { - ulong position = context.Request.ReceiveBuff[0].Position; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; + ulong bufferLen = context.Request.ReceiveBuff[0].Size; ReadOption readOption = context.RequestData.ReadStruct(); context.RequestData.BaseStream.Position += 4; @@ -27,15 +28,14 @@ namespace Ryujinx.HLE.HOS.Services.Fs.FileSystemProxy long offset = context.RequestData.ReadInt64(); long size = context.RequestData.ReadInt64(); - byte[] data = new byte[context.Request.ReceiveBuff[0].Size]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _baseFile.Get.Read(out long bytesRead, offset, new OutBuffer(region.Memory.Span), size, readOption); - Result result = _baseFile.Get.Read(out long bytesRead, offset, new OutBuffer(data), size, readOption); + context.ResponseData.Write(bytesRead); - context.Memory.Write(position, data); - - context.ResponseData.Write(bytesRead); - - return (ResultCode)result.Value; + return (ResultCode)result.Value; + } } [CommandHipc(1)] diff --git a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFileSystem.cs b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFileSystem.cs index a551d163f..d68ef3952 100644 --- a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFileSystem.cs +++ b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IFileSystem.cs @@ -197,13 +197,7 @@ namespace Ryujinx.HLE.HOS.Services.Fs.FileSystemProxy context.ResponseData.Write(timestamp.Created); context.ResponseData.Write(timestamp.Modified); context.ResponseData.Write(timestamp.Accessed); - - byte[] data = new byte[8]; - - // is valid? - data[0] = 1; - - context.ResponseData.Write(data); + context.ResponseData.Write(1L); // Is valid? return (ResultCode)result.Value; } diff --git a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IStorage.cs b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IStorage.cs index 9dbfd2b0d..5359cadd7 100644 --- a/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IStorage.cs +++ b/Ryujinx.HLE/HOS/Services/Fs/FileSystemProxy/IStorage.cs @@ -23,21 +23,21 @@ namespace Ryujinx.HLE.HOS.Services.Fs.FileSystemProxy if (context.Request.ReceiveBuff.Count > 0) { - IpcBuffDesc buffDesc = context.Request.ReceiveBuff[0]; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; + ulong bufferLen = context.Request.ReceiveBuff[0].Size; // Use smaller length to avoid overflows. - if (size > buffDesc.Size) + if (size > bufferLen) { - size = buffDesc.Size; + size = bufferLen; } - byte[] data = new byte[size]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _baseStorage.Get.Read((long)offset, new OutBuffer(region.Memory.Span), (long)size); - Result result = _baseStorage.Get.Read((long)offset, new OutBuffer(data), (long)size); - - context.Memory.Write(buffDesc.Position, data); - - return (ResultCode)result.Value; + return (ResultCode)result.Value; + } } return ResultCode.Success; diff --git a/Ryujinx.HLE/HOS/Services/Fs/IFileSystemProxy.cs b/Ryujinx.HLE/HOS/Services/Fs/IFileSystemProxy.cs index 01e1aa34d..c32750be1 100644 --- a/Ryujinx.HLE/HOS/Services/Fs/IFileSystemProxy.cs +++ b/Ryujinx.HLE/HOS/Services/Fs/IFileSystemProxy.cs @@ -500,16 +500,16 @@ namespace Ryujinx.HLE.HOS.Services.Fs SaveDataSpaceId spaceId = (SaveDataSpaceId)context.RequestData.ReadInt64(); SaveDataFilter filter = context.RequestData.ReadStruct(); - ulong bufferPosition = context.Request.ReceiveBuff[0].Position; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; ulong bufferLen = context.Request.ReceiveBuff[0].Size; - byte[] infoBuffer = new byte[bufferLen]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _baseFileSystemProxy.Get.FindSaveDataWithFilter(out long count, new OutBuffer(region.Memory.Span), spaceId, in filter); + if (result.IsFailure()) return (ResultCode)result.Value; - Result result = _baseFileSystemProxy.Get.FindSaveDataWithFilter(out long count, new OutBuffer(infoBuffer), spaceId, in filter); - if (result.IsFailure()) return (ResultCode)result.Value; - - context.Memory.Write(bufferPosition, infoBuffer); - context.ResponseData.Write(count); + context.ResponseData.Write(count); + } return ResultCode.Success; } diff --git a/Ryujinx.HLE/HOS/Services/Fs/ISaveDataInfoReader.cs b/Ryujinx.HLE/HOS/Services/Fs/ISaveDataInfoReader.cs index bb664e55f..0bb4123f2 100644 --- a/Ryujinx.HLE/HOS/Services/Fs/ISaveDataInfoReader.cs +++ b/Ryujinx.HLE/HOS/Services/Fs/ISaveDataInfoReader.cs @@ -17,17 +17,17 @@ namespace Ryujinx.HLE.HOS.Services.Fs // ReadSaveDataInfo() -> (u64, buffer) public ResultCode ReadSaveDataInfo(ServiceCtx context) { - ulong bufferPosition = context.Request.ReceiveBuff[0].Position; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; ulong bufferLen = context.Request.ReceiveBuff[0].Size; - byte[] infoBuffer = new byte[bufferLen]; + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Result result = _baseReader.Get.Read(out long readCount, new OutBuffer(region.Memory.Span)); - Result result = _baseReader.Get.Read(out long readCount, new OutBuffer(infoBuffer)); + context.ResponseData.Write(readCount); - context.Memory.Write(bufferPosition, infoBuffer); - context.ResponseData.Write(readCount); - - return (ResultCode)result.Value; + return (ResultCode)result.Value; + } } protected override void Dispose(bool isDisposing) diff --git a/Ryujinx.HLE/HOS/Services/Ssl/SslService/ISslConnection.cs b/Ryujinx.HLE/HOS/Services/Ssl/SslService/ISslConnection.cs index 96ae84be7..ff6586734 100644 --- a/Ryujinx.HLE/HOS/Services/Ssl/SslService/ISslConnection.cs +++ b/Ryujinx.HLE/HOS/Services/Ssl/SslService/ISslConnection.cs @@ -142,14 +142,13 @@ namespace Ryujinx.HLE.HOS.Services.Ssl.SslService // GetHostName(buffer) -> u32 public ResultCode GetHostName(ServiceCtx context) { - ulong hostNameDataPosition = context.Request.ReceiveBuff[0].Position; - ulong hostNameDataSize = context.Request.ReceiveBuff[0].Size; + ulong bufferAddress = context.Request.ReceiveBuff[0].Position; + ulong bufferLen = context.Request.ReceiveBuff[0].Size; - byte[] hostNameData = new byte[hostNameDataSize]; - - Encoding.ASCII.GetBytes(_hostName, hostNameData); - - context.Memory.Write(hostNameDataPosition, hostNameData); + using (var region = context.Memory.GetWritableRegion(bufferAddress, (int)bufferLen, true)) + { + Encoding.ASCII.GetBytes(_hostName, region.Memory.Span); + } context.ResponseData.Write((uint)_hostName.Length);