From ce69201f3398facedd960bb09264e4ef5bf6a672 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 10 Jun 2018 14:32:33 -0400 Subject: [PATCH 1/2] Common/Network: Get rid of out parameters for MAC address utilities Given we have std::array and std::optional, we can use these in conjunction with one another to avoid the need for out parameters. --- Source/Core/Common/Network.cpp | 66 +++++++++++-------- Source/Core/Common/Network.h | 10 ++- .../Core/Core/HW/EXI/EXI_DeviceEthernet.cpp | 12 ++-- Source/Core/Core/IOS/Network/IP/Top.cpp | 7 +- Source/Core/Core/IOS/Network/MACUtils.cpp | 18 +++-- Source/Core/Core/IOS/Network/MACUtils.h | 4 +- Source/Core/Core/IOS/Network/NCD/Manage.cpp | 7 +- Source/Core/Core/IOS/Network/WD/Command.cpp | 6 +- Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp | 5 +- 9 files changed, 77 insertions(+), 58 deletions(-) diff --git a/Source/Core/Common/Network.cpp b/Source/Core/Common/Network.cpp index 1d95e0c8a4..acfb60e55c 100644 --- a/Source/Core/Common/Network.cpp +++ b/Source/Core/Common/Network.cpp @@ -2,67 +2,75 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include "Common/Network.h" + +#include #include #include #include -#include "Common/Network.h" #include "Common/Random.h" #include "Common/StringUtil.h" namespace Common { -void GenerateMacAddress(const MACConsumer type, u8* mac) +MACAddress GenerateMacAddress(const MACConsumer type) { - memset(mac, 0, MAC_ADDRESS_SIZE); + constexpr std::array oui_bba{{0x00, 0x09, 0xbf}}; + constexpr std::array oui_ios{{0x00, 0x17, 0xab}}; - u8 const oui_bba[] = {0x00, 0x09, 0xbf}; - u8 const oui_ios[] = {0x00, 0x17, 0xab}; + MACAddress mac{}; switch (type) { case MACConsumer::BBA: - memcpy(mac, oui_bba, 3); + std::copy(oui_bba.begin(), oui_bba.end(), mac.begin()); break; case MACConsumer::IOS: - memcpy(mac, oui_ios, 3); + std::copy(oui_ios.begin(), oui_ios.end(), mac.begin()); break; } // Generate the 24-bit NIC-specific portion of the MAC address. - Common::Random::Generate(&mac[3], 3); + Random::Generate(&mac[3], 3); + return mac; } -std::string MacAddressToString(const u8* mac) +std::string MacAddressToString(const MACAddress& mac) { return StringFromFormat("%02x:%02x:%02x:%02x:%02x:%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); } -bool StringToMacAddress(const std::string& mac_string, u8* mac) +std::optional StringToMacAddress(const std::string& mac_string) { - bool success = false; - if (!mac_string.empty()) - { - int x = 0; - memset(mac, 0, MAC_ADDRESS_SIZE); + if (mac_string.empty()) + return {}; - for (size_t i = 0; i < mac_string.size() && x < (MAC_ADDRESS_SIZE * 2); ++i) + int x = 0; + MACAddress mac{}; + + for (size_t i = 0; i < mac_string.size() && x < (MAC_ADDRESS_SIZE * 2); ++i) + { + char c = tolower(mac_string.at(i)); + if (c >= '0' && c <= '9') { - char c = tolower(mac_string.at(i)); - if (c >= '0' && c <= '9') - { - mac[x / 2] |= (c - '0') << ((x & 1) ? 0 : 4); - ++x; - } - else if (c >= 'a' && c <= 'f') - { - mac[x / 2] |= (c - 'a' + 10) << ((x & 1) ? 0 : 4); - ++x; - } + mac[x / 2] |= (c - '0') << ((x & 1) ? 0 : 4); + ++x; + } + else if (c >= 'a' && c <= 'f') + { + mac[x / 2] |= (c - 'a' + 10) << ((x & 1) ? 0 : 4); + ++x; } - success = x / 2 == MAC_ADDRESS_SIZE; } - return success; + + // A valid 48-bit MAC address consists of 6 octets, where each + // nibble is a character in the MAC address, making 12 characters + // in total. + if (x / 2 != MAC_ADDRESS_SIZE) + return {}; + + return std::make_optional(mac); } } // namespace Common diff --git a/Source/Core/Common/Network.h b/Source/Core/Common/Network.h index 8353d84468..3f3c343fe8 100644 --- a/Source/Core/Common/Network.h +++ b/Source/Core/Common/Network.h @@ -4,6 +4,8 @@ #pragma once +#include +#include #include #include "Common/CommonTypes.h" @@ -21,7 +23,9 @@ enum MAC_ADDRESS_SIZE = 6 }; -void GenerateMacAddress(const MACConsumer type, u8* mac); -std::string MacAddressToString(const u8* mac); -bool StringToMacAddress(const std::string& mac_string, u8* mac); +using MACAddress = std::array; + +MACAddress GenerateMacAddress(MACConsumer type); +std::string MacAddressToString(const MACAddress& mac); +std::optional StringToMacAddress(const std::string& mac_string); } // namespace Common diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp index 42d1bddd8f..1972e3e3ea 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp @@ -5,6 +5,7 @@ #include "Core/HW/EXI/EXI_DeviceEthernet.h" #include +#include #include #include "Common/ChunkFile.h" @@ -33,16 +34,17 @@ CEXIETHERNET::CEXIETHERNET() // Parse MAC address from config, and generate a new one if it doesn't // exist or can't be parsed. std::string& mac_addr_setting = SConfig::GetInstance().m_bba_mac; - u8 mac_addr[Common::MAC_ADDRESS_SIZE] = {0}; + std::optional mac_addr = Common::StringToMacAddress(mac_addr_setting); - if (!Common::StringToMacAddress(mac_addr_setting, mac_addr)) + if (!mac_addr) { - Common::GenerateMacAddress(Common::MACConsumer::BBA, mac_addr); - mac_addr_setting = Common::MacAddressToString(mac_addr); + mac_addr = Common::GenerateMacAddress(Common::MACConsumer::BBA); + mac_addr_setting = Common::MacAddressToString(mac_addr.value()); SConfig::GetInstance().SaveSettings(); } - memcpy(&mBbaMem[BBA_NAFR_PAR0], mac_addr, Common::MAC_ADDRESS_SIZE); + const auto& mac = mac_addr.value(); + memcpy(&mBbaMem[BBA_NAFR_PAR0], mac.data(), mac.size()); // HACK: .. fully established 100BASE-T link mBbaMem[BBA_NWAYS] = NWAYS_LS100 | NWAYS_LPNWAY | NWAYS_100TXF | NWAYS_ANCLPT; diff --git a/Source/Core/Core/IOS/Network/IP/Top.cpp b/Source/Core/Core/IOS/Network/IP/Top.cpp index dced159b66..fc247fe168 100644 --- a/Source/Core/Core/IOS/Network/IP/Top.cpp +++ b/Source/Core/Core/IOS/Network/IP/Top.cpp @@ -878,10 +878,11 @@ IPCCommandResult NetIPTop::HandleGetInterfaceOptRequest(const IOCtlVRequest& req break; case 0x1004: // mac address - u8 address[Common::MAC_ADDRESS_SIZE]; - IOS::Net::GetMACAddress(address); - Memory::CopyToEmu(request.io_vectors[0].address, address, sizeof(address)); + { + const Common::MACAddress address = IOS::Net::GetMACAddress(); + Memory::CopyToEmu(request.io_vectors[0].address, address.data(), address.size()); break; + } case 0x1005: // link state Memory::Write_U32(1, request.io_vectors[0].address); diff --git a/Source/Core/Core/IOS/Network/MACUtils.cpp b/Source/Core/Core/IOS/Network/MACUtils.cpp index 84b6e521d5..6df31c5d08 100644 --- a/Source/Core/Core/IOS/Network/MACUtils.cpp +++ b/Source/Core/Core/IOS/Network/MACUtils.cpp @@ -4,6 +4,7 @@ #include "Core/IOS/Network/MACUtils.h" +#include #include #include "Common/CommonTypes.h" @@ -15,13 +16,13 @@ namespace IOS::Net { -static void SaveMACAddress(const u8* mac) +static void SaveMACAddress(const Common::MACAddress& mac) { SConfig::GetInstance().m_WirelessMac = Common::MacAddressToString(mac); SConfig::GetInstance().SaveSettings(); } -void GetMACAddress(u8* mac) +Common::MACAddress GetMACAddress() { // Parse MAC address from config, and generate a new one if it doesn't // exist or can't be parsed. @@ -30,19 +31,22 @@ void GetMACAddress(u8* mac) if (Core::WantsDeterminism()) wireless_mac = "12:34:56:78:9a:bc"; - if (!Common::StringToMacAddress(wireless_mac, mac)) + std::optional mac = Common::StringToMacAddress(wireless_mac); + + if (!mac) { - Common::GenerateMacAddress(Common::MACConsumer::IOS, mac); - SaveMACAddress(mac); + mac = Common::GenerateMacAddress(Common::MACConsumer::IOS); + SaveMACAddress(mac.value()); if (!wireless_mac.empty()) { ERROR_LOG(IOS_NET, "The MAC provided (%s) is invalid. We have " "generated another one for you.", - Common::MacAddressToString(mac).c_str()); + Common::MacAddressToString(mac.value()).c_str()); } } - INFO_LOG(IOS_NET, "Using MAC address: %s", Common::MacAddressToString(mac).c_str()); + INFO_LOG(IOS_NET, "Using MAC address: %s", Common::MacAddressToString(mac.value()).c_str()); + return mac.value(); } } // namespace IOS::Net diff --git a/Source/Core/Core/IOS/Network/MACUtils.h b/Source/Core/Core/IOS/Network/MACUtils.h index 409e6cce52..2ec3fa3335 100644 --- a/Source/Core/Core/IOS/Network/MACUtils.h +++ b/Source/Core/Core/IOS/Network/MACUtils.h @@ -4,9 +4,9 @@ #pragma once -#include "Common/CommonTypes.h" +#include "Common/Network.h" namespace IOS::Net { -void GetMACAddress(u8* mac); +Common::MACAddress GetMACAddress(); } // namespace IOS::Net diff --git a/Source/Core/Core/IOS/Network/NCD/Manage.cpp b/Source/Core/Core/IOS/Network/NCD/Manage.cpp index ea8ad0c77a..8bb003eb92 100644 --- a/Source/Core/Core/IOS/Network/NCD/Manage.cpp +++ b/Source/Core/Core/IOS/Network/NCD/Manage.cpp @@ -65,12 +65,13 @@ IPCCommandResult NetNCDManage::IOCtlV(const IOCtlVRequest& request) break; case IOCTLV_NCD_GETWIRELESSMACADDRESS: + { INFO_LOG(IOS_NET, "NET_NCD_MANAGE: IOCTLV_NCD_GETWIRELESSMACADDRESS"); - u8 address[Common::MAC_ADDRESS_SIZE]; - IOS::Net::GetMACAddress(address); - Memory::CopyToEmu(request.io_vectors.at(1).address, address, sizeof(address)); + const Common::MACAddress address = IOS::Net::GetMACAddress(); + Memory::CopyToEmu(request.io_vectors.at(1).address, address.data(), address.size()); break; + } default: INFO_LOG(IOS_NET, "NET_NCD_MANAGE IOCtlV: %#x", request.request); diff --git a/Source/Core/Core/IOS/Network/WD/Command.cpp b/Source/Core/Core/IOS/Network/WD/Command.cpp index f93280da41..a2dfee49a7 100644 --- a/Source/Core/Core/IOS/Network/WD/Command.cpp +++ b/Source/Core/Core/IOS/Network/WD/Command.cpp @@ -4,6 +4,7 @@ #include "Core/IOS/Network/WD/Command.h" +#include #include #include @@ -65,9 +66,8 @@ IPCCommandResult NetWDCommand::IOCtlV(const IOCtlVRequest& request) memcpy(info->country, "US", 2); info->ntr_allowed_channels = Common::swap16(0xfffe); - u8 address[Common::MAC_ADDRESS_SIZE]; - IOS::Net::GetMACAddress(address); - memcpy(info->mac, address, sizeof(info->mac)); + const Common::MACAddress address = IOS::Net::GetMACAddress(); + std::copy(address.begin(), address.end(), info->mac); } break; diff --git a/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp b/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp index f5d3014f3d..174d0bb0da 100644 --- a/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp +++ b/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp @@ -514,8 +514,7 @@ void BluetoothReal::LoadLinkKeys() if (index == std::string::npos) continue; - bdaddr_t address; - Common::StringToMacAddress(pair.substr(0, index), address.data()); + bdaddr_t address = Common::StringToMacAddress(pair.substr(0, index)).value(); std::reverse(address.begin(), address.end()); const std::string& key_string = pair.substr(index + 1); @@ -540,7 +539,7 @@ void BluetoothReal::SaveLinkKeys() bdaddr_t address; // Reverse the address so that it is stored in the correct order in the config file std::reverse_copy(entry.first.begin(), entry.first.end(), address.begin()); - oss << Common::MacAddressToString(address.data()); + oss << Common::MacAddressToString(address); oss << '='; oss << std::hex; for (const u16& data : entry.second) From 59846378b3c7ffb427718c6a2375873a73909b69 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 10 Jun 2018 16:02:49 -0400 Subject: [PATCH 2/2] BTReal: Handle case where a link key may be invalid within LoadLinkKeys() This can only occur if a user purposely corrupts their config file, but still, we may as well protect users from themselves. --- Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp b/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp index 174d0bb0da..243493d5f1 100644 --- a/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp +++ b/Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp @@ -514,8 +514,17 @@ void BluetoothReal::LoadLinkKeys() if (index == std::string::npos) continue; - bdaddr_t address = Common::StringToMacAddress(pair.substr(0, index)).value(); - std::reverse(address.begin(), address.end()); + const std::string address_string = pair.substr(0, index); + std::optional address = Common::StringToMacAddress(address_string); + if (!address) + { + ERROR_LOG(IOS_WIIMOTE, "Malformed MAC address (%s). Skipping loading of current link key.", + address_string.c_str()); + continue; + } + + auto& mac = address.value(); + std::reverse(mac.begin(), mac.end()); const std::string& key_string = pair.substr(index + 1); linkkey_t key{}; @@ -527,7 +536,7 @@ void BluetoothReal::LoadLinkKeys() key[pos++] = value; } - m_link_keys[address] = key; + m_link_keys[mac] = key; } }