Skip to content

Commit

Permalink
IOS: Remove calls to GetPointer
Browse files Browse the repository at this point in the history
Typically when someone uses GetPointer, it's because they want to read
from a range of memory. GetPointer is unsafe to use for this. While it
does check that the passed-in address is valid, it doesn't know the size
of the range that will be accessed, so it can't check that the end
address is valid. The safer alternative GetPointerForRange should be
used instead.

Note that there is still the problem of many callers not checking for
nullptr.

This is the first part of a series of changes that will remove the usage
of GetPointer in different parts of the code base. This commit gets rid
of every GetPointer call from our IOS code except for a particularly
tricky one in BluetoothEmuDevice.
  • Loading branch information
JosJuice committed Mar 31, 2024
1 parent f30d3c9 commit 9873bcf
Show file tree
Hide file tree
Showing 17 changed files with 128 additions and 103 deletions.
2 changes: 2 additions & 0 deletions Source/Core/Core/HW/Memmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ class MemoryManager
// Routines to access physically addressed memory, designed for use by
// emulated hardware outside the CPU. Use "Device_" prefix.
std::string GetString(u32 em_address, size_t size = 0);
// WARNING: Incrementing the pointer returned by GetPointer is unsafe without additional bounds
// checks. New code should use other functions instead, like GetPointerForRange or CopyFromEmu.
u8* GetPointer(u32 address) const;
u8* GetPointerForRange(u32 address, size_t size) const;
void CopyFromEmu(void* data, u32 address, size_t size) const;
Expand Down
6 changes: 3 additions & 3 deletions Source/Core/Core/IOS/Device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ void IOCtlRequest::Dump(Core::System& system, const std::string& description,

Log("===== " + description, type, level);
GENERIC_LOG_FMT(type, level, "In buffer\n{}",
HexDump(memory.GetPointer(buffer_in), buffer_in_size));
HexDump(memory.GetPointerForRange(buffer_in, buffer_in_size), buffer_in_size));
GENERIC_LOG_FMT(type, level, "Out buffer\n{}",
HexDump(memory.GetPointer(buffer_out), buffer_out_size));
HexDump(memory.GetPointerForRange(buffer_out, buffer_out_size), buffer_out_size));
}

void IOCtlRequest::DumpUnknown(Core::System& system, const std::string& description,
Expand All @@ -139,7 +139,7 @@ void IOCtlVRequest::Dump(Core::System& system, std::string_view description,
for (const auto& vector : in_vectors)
{
GENERIC_LOG_FMT(type, level, "in[{}] (size={:#x}):\n{}", i++, vector.size,
HexDump(memory.GetPointer(vector.address), vector.size));
HexDump(memory.GetPointerForRange(vector.address, vector.size), vector.size));
}

i = 0;
Expand Down
3 changes: 2 additions & 1 deletion Source/Core/Core/IOS/ES/ES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,8 @@ IPCReply ESDevice::SetUpStreamKey(const Context& context, const IOCtlVRequest& r

u32 handle;
const ReturnCode ret = m_core.SetUpStreamKey(
context.uid, memory.GetPointer(request.in_vectors[0].address), tmd, &handle);
context.uid, memory.GetPointerForRange(request.in_vectors[0].address, sizeof(ES::TicketView)),
tmd, &handle);
memory.Write_U32(handle, request.io_vectors[0].address);
return IPCReply(ret);
}
Expand Down
19 changes: 10 additions & 9 deletions Source/Core/Core/IOS/ES/Identity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ IPCReply ESDevice::Encrypt(u32 uid, const IOCtlVRequest& request)
auto& system = GetSystem();
auto& memory = system.GetMemory();
u32 keyIndex = memory.Read_U32(request.in_vectors[0].address);
u8* source = memory.GetPointer(request.in_vectors[2].address);
u32 size = request.in_vectors[2].size;
u8* iv = memory.GetPointer(request.io_vectors[0].address);
u8* destination = memory.GetPointer(request.io_vectors[1].address);
u8* source = memory.GetPointerForRange(request.in_vectors[2].address, size);
u8* iv = memory.GetPointerForRange(request.io_vectors[0].address, 16);
u8* destination = memory.GetPointerForRange(request.io_vectors[1].address, size);

// TODO: Check whether the active title is allowed to encrypt.

Expand All @@ -71,10 +71,10 @@ IPCReply ESDevice::Decrypt(u32 uid, const IOCtlVRequest& request)
auto& system = GetSystem();
auto& memory = system.GetMemory();
u32 keyIndex = memory.Read_U32(request.in_vectors[0].address);
u8* source = memory.GetPointer(request.in_vectors[2].address);
u32 size = request.in_vectors[2].size;
u8* iv = memory.GetPointer(request.io_vectors[0].address);
u8* destination = memory.GetPointer(request.io_vectors[1].address);
u8* source = memory.GetPointerForRange(request.in_vectors[2].address, size);
u8* iv = memory.GetPointerForRange(request.io_vectors[0].address, 16);
u8* destination = memory.GetPointerForRange(request.io_vectors[1].address, size);

// TODO: Check whether the active title is allowed to decrypt.

Expand Down Expand Up @@ -118,10 +118,11 @@ IPCReply ESDevice::Sign(const IOCtlVRequest& request)
INFO_LOG_FMT(IOS_ES, "IOCTL_ES_SIGN");
auto& system = GetSystem();
auto& memory = system.GetMemory();
u8* ap_cert_out = memory.GetPointer(request.io_vectors[1].address);
u8* data = memory.GetPointer(request.in_vectors[0].address);
u8* ap_cert_out = memory.GetPointerForRange(request.io_vectors[1].address, sizeof(CertECC));
u32 data_size = request.in_vectors[0].size;
u8* sig_out = memory.GetPointer(request.io_vectors[0].address);
u8* data = memory.GetPointerForRange(request.in_vectors[0].address, data_size);
u8* sig_out =
memory.GetPointerForRange(request.io_vectors[0].address, sizeof(Common::ec::Signature));

if (!m_core.m_title_context.active)
return IPCReply(ES_EINVAL);
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/Core/IOS/ES/TitleContents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ IPCReply ESDevice::ReadContent(u32 uid, const IOCtlVRequest& request)

INFO_LOG_FMT(IOS_ES, "ReadContent(uid={:#x}, cfd={}, size={}, addr={:08x})", uid, cfd, size,
addr);
return m_core.ReadContent(cfd, memory.GetPointer(addr), size, uid, ticks);
return m_core.ReadContent(cfd, memory.GetPointerForRange(addr, size), size, uid, ticks);
});
}

Expand Down
13 changes: 7 additions & 6 deletions Source/Core/Core/IOS/ES/TitleManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,9 @@ IPCReply ESDevice::ImportContentData(Context& context, const IOCtlVRequest& requ
auto& memory = system.GetMemory();

u32 content_fd = memory.Read_U32(request.in_vectors[0].address);
u8* data_start = memory.GetPointer(request.in_vectors[1].address);
return IPCReply(
m_core.ImportContentData(context, content_fd, data_start, request.in_vectors[1].size));
u32 data_size = request.in_vectors[1].size;
u8* data_start = memory.GetPointerForRange(request.in_vectors[1].address, data_size);
return IPCReply(m_core.ImportContentData(context, content_fd, data_start, data_size));
}

static bool CheckIfContentHashMatches(const std::vector<u8>& content, const ES::Content& info)
Expand Down Expand Up @@ -630,7 +630,8 @@ IPCReply ESDevice::DeleteTicket(const IOCtlVRequest& request)

auto& system = GetSystem();
auto& memory = system.GetMemory();
return IPCReply(m_core.DeleteTicket(memory.GetPointer(request.in_vectors[0].address)));
return IPCReply(m_core.DeleteTicket(
memory.GetPointerForRange(request.in_vectors[0].address, sizeof(ES::TicketView))));
}

ReturnCode ESCore::DeleteTitleContent(u64 title_id) const
Expand Down Expand Up @@ -732,8 +733,8 @@ IPCReply ESDevice::ExportTitleInit(Context& context, const IOCtlVRequest& reques
auto& memory = system.GetMemory();

const u64 title_id = memory.Read_U64(request.in_vectors[0].address);
u8* tmd_bytes = memory.GetPointer(request.io_vectors[0].address);
const u32 tmd_size = request.io_vectors[0].size;
u8* tmd_bytes = memory.GetPointerForRange(request.io_vectors[0].address, tmd_size);

return IPCReply(m_core.ExportTitleInit(context, title_id, tmd_bytes, tmd_size,
m_core.m_title_context.tmd.GetTitleId(),
Expand Down Expand Up @@ -832,8 +833,8 @@ IPCReply ESDevice::ExportContentData(Context& context, const IOCtlVRequest& requ
auto& memory = system.GetMemory();

const u32 content_fd = memory.Read_U32(request.in_vectors[0].address);
u8* data = memory.GetPointer(request.io_vectors[0].address);
const u32 bytes_to_read = request.io_vectors[0].size;
u8* data = memory.GetPointerForRange(request.io_vectors[0].address, bytes_to_read);

return IPCReply(m_core.ExportContentData(context, content_fd, data, bytes_to_read));
}
Expand Down
18 changes: 10 additions & 8 deletions Source/Core/Core/IOS/ES/Views.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ IPCReply ESDevice::GetV0TicketFromView(const IOCtlVRequest& request)

auto& system = GetSystem();
auto& memory = system.GetMemory();
return IPCReply(m_core.GetTicketFromView(memory.GetPointer(request.in_vectors[0].address),
memory.GetPointer(request.io_vectors[0].address),
nullptr, 0));
return IPCReply(m_core.GetTicketFromView(
memory.GetPointerForRange(request.in_vectors[0].address, sizeof(ES::TicketView)),
memory.GetPointerForRange(request.io_vectors[0].address, sizeof(ES::Ticket)), nullptr, 0));
}

IPCReply ESDevice::GetTicketSizeFromView(const IOCtlVRequest& request)
Expand All @@ -179,8 +179,9 @@ IPCReply ESDevice::GetTicketSizeFromView(const IOCtlVRequest& request)

auto& system = GetSystem();
auto& memory = system.GetMemory();
const ReturnCode ret = m_core.GetTicketFromView(memory.GetPointer(request.in_vectors[0].address),
nullptr, &ticket_size, std::nullopt);
const ReturnCode ret = m_core.GetTicketFromView(
memory.GetPointerForRange(request.in_vectors[0].address, sizeof(ES::TicketView)), nullptr,
&ticket_size, std::nullopt);
memory.Write_U32(ticket_size, request.io_vectors[0].address);
return IPCReply(ret);
}
Expand All @@ -201,9 +202,10 @@ IPCReply ESDevice::GetTicketFromView(const IOCtlVRequest& request)
if (ticket_size != request.io_vectors[0].size)
return IPCReply(ES_EINVAL);

return IPCReply(m_core.GetTicketFromView(memory.GetPointer(request.in_vectors[0].address),
memory.GetPointer(request.io_vectors[0].address),
&ticket_size, std::nullopt));
return IPCReply(m_core.GetTicketFromView(
memory.GetPointerForRange(request.in_vectors[0].address, sizeof(ES::TicketView)),
memory.GetPointerForRange(request.io_vectors[0].address, ticket_size), &ticket_size,
std::nullopt));
}

IPCReply ESDevice::GetTMDViewSize(const IOCtlVRequest& request)
Expand Down
8 changes: 4 additions & 4 deletions Source/Core/Core/IOS/FS/FileSystemProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ std::optional<IPCReply> FSDevice::Read(const ReadWriteRequest& request)
return MakeIPCReply([&](Ticks t) {
auto& system = GetSystem();
auto& memory = system.GetMemory();
return m_core.Read(request.fd, memory.GetPointer(request.buffer), request.size, request.buffer,
t);
return m_core.Read(request.fd, memory.GetPointerForRange(request.buffer, request.size),
request.size, request.buffer, t);
});
}

Expand Down Expand Up @@ -357,8 +357,8 @@ std::optional<IPCReply> FSDevice::Write(const ReadWriteRequest& request)
return MakeIPCReply([&](Ticks t) {
auto& system = GetSystem();
auto& memory = system.GetMemory();
return m_core.Write(request.fd, memory.GetPointer(request.buffer), request.size, request.buffer,
t);
return m_core.Write(request.fd, memory.GetPointerForRange(request.buffer, request.size),
request.size, request.buffer, t);
});
}

Expand Down
29 changes: 18 additions & 11 deletions Source/Core/Core/IOS/Network/IP/Top.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "Core/IOS/Network/IP/Top.h"

#include <algorithm>
#include <array>
#include <cstddef>
#include <cstring>
#include <memory>
Expand All @@ -20,6 +21,7 @@
#include <fmt/format.h>

#include "Common/Assert.h"
#include "Common/BitUtils.h"
#include "Common/CommonTypes.h"
#include "Common/Logging/Log.h"
#include "Common/Network.h"
Expand Down Expand Up @@ -80,12 +82,12 @@ void NetIPTopDevice::DoState(PointerWrap& p)
Device::DoState(p);
}

static int inet_pton(const char* src, unsigned char* dst)
static std::optional<u32> inet_pton(const char* src)
{
int saw_digit = 0;
int octets = 0;
unsigned char tmp[4]{};
unsigned char* tp = tmp;
std::array<unsigned char, 4> tmp{};
unsigned char* tp = tmp.data();
char ch;

while ((ch = *src++) != '\0')
Expand All @@ -95,31 +97,30 @@ static int inet_pton(const char* src, unsigned char* dst)
unsigned int newt = (*tp * 10) + (ch - '0');

if (newt > 255)
return 0;
return std::nullopt;
*tp = newt;
if (!saw_digit)
{
if (++octets > 4)
return 0;
return std::nullopt;
saw_digit = 1;
}
}
else if (ch == '.' && saw_digit)
{
if (octets == 4)
return 0;
return std::nullopt;
*++tp = 0;
saw_digit = 0;
}
else
{
return 0;
return std::nullopt;
}
}
if (octets < 4)
return 0;
memcpy(dst, tmp, 4);
return 1;
return std::nullopt;
return Common::BitCast<u32>(tmp);
}

// Maps SOCKOPT level from Wii to native
Expand Down Expand Up @@ -655,7 +656,13 @@ IPCReply NetIPTopDevice::HandleInetPToNRequest(const IOCtlRequest& request)

const std::string address = memory.GetString(request.buffer_in);
INFO_LOG_FMT(IOS_NET, "IOCTL_SO_INETPTON (Translating: {})", address);
return IPCReply(inet_pton(address.c_str(), memory.GetPointer(request.buffer_out + 4)));

const std::optional<u32> result = inet_pton(address.c_str());
if (!result)
return IPCReply(0);

memory.CopyToEmu(request.buffer_out + 4, &*result, sizeof(u32));
return IPCReply(1);
}

IPCReply NetIPTopDevice::HandleInetNToPRequest(const IOCtlRequest& request)
Expand Down
7 changes: 4 additions & 3 deletions Source/Core/Core/IOS/Network/SSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,14 @@ std::optional<IPCReply> NetSSLDevice::IOCtlV(const IOCtlVRequest& request)
if (IsSSLIDValid(sslID))
{
WII_SSL* ssl = &_SSL[sslID];
int ret =
mbedtls_x509_crt_parse_der(&ssl->cacert, memory.GetPointer(BufferOut2), BufferOutSize2);
int ret = mbedtls_x509_crt_parse_der(
&ssl->cacert, memory.GetPointerForRange(BufferOut2, BufferOutSize2), BufferOutSize2);

if (Config::Get(Config::MAIN_NETWORK_SSL_DUMP_ROOT_CA))
{
std::string filename = File::GetUserPath(D_DUMPSSL_IDX) + ssl->hostname + "_rootca.der";
File::IOFile(filename, "wb").WriteBytes(memory.GetPointer(BufferOut2), BufferOutSize2);
File::IOFile(filename, "wb")
.WriteBytes(memory.GetPointerForRange(BufferOut2, BufferOutSize2), BufferOutSize2);
}

if (ret)
Expand Down

0 comments on commit 9873bcf

Please sign in to comment.