From f5e5dcae3330c2c95ae8a75b710db3ee18495b0c Mon Sep 17 00:00:00 2001 From: Claudio DeSouza Date: Mon, 14 Oct 2024 23:41:39 +0100 Subject: [PATCH] [CodeHealth] Use span of bytes to read arbitrary data This PR touches in different places of the code base where the use of `reinterpret_cast` was necessary to allow data to be handled in different format. Span is both safer and more ergonimic. It is preferrable over `void*`, and particularly we should avoid any type of casts involving byte values to numeric values, as those can cause undefined behaviour. This change additionally refactors `DeviceIdImpl::IsValidMacAddress` to use `base::MakeFixedFlatSet` for the set of invalid mac addresses we were searching for, as the search was sequential, and for successful values it meant visiting every element in the array to check for comparison. To make sure the binary search works as expected, a test has been added. --- browser/brave_ads/device_id/BUILD.gn | 11 + browser/brave_ads/device_id/device_id_impl.cc | 299 ++++++++++-------- browser/brave_ads/device_id/device_id_impl.h | 11 +- .../device_id/device_id_impl_linux.cc | 12 +- .../brave_ads/device_id/device_id_impl_mac.cc | 13 +- .../device_id/device_id_impl_unittest.cc | 149 +++++++++ .../brave_ads/device_id/device_id_impl_win.cc | 9 +- browser/default_protocol_handler_utils_win.cc | 85 ++--- .../content/browser/brave_farbling_service.cc | 11 +- test/BUILD.gn | 1 + 10 files changed, 412 insertions(+), 189 deletions(-) create mode 100644 browser/brave_ads/device_id/device_id_impl_unittest.cc diff --git a/browser/brave_ads/device_id/BUILD.gn b/browser/brave_ads/device_id/BUILD.gn index 99fa841cdf9d..af6cdfcf41ed 100644 --- a/browser/brave_ads/device_id/BUILD.gn +++ b/browser/brave_ads/device_id/BUILD.gn @@ -37,3 +37,14 @@ source_set("device_id") { libs += [ "iphlpapi.lib" ] } } + +source_set("unit_tests") { + testonly = true + sources = [ "device_id_impl_unittest.cc" ] + + deps = [ + ":device_id", + "//base/test:test_support", + "//testing/gtest", + ] +} diff --git a/browser/brave_ads/device_id/device_id_impl.cc b/browser/brave_ads/device_id/device_id_impl.cc index 7ba6ddefac09..c96cda4d6d9d 100644 --- a/browser/brave_ads/device_id/device_id_impl.cc +++ b/browser/brave_ads/device_id/device_id_impl.cc @@ -5,6 +5,7 @@ #include "brave/browser/brave_ads/device_id/device_id_impl.h" +#include #include #include #include @@ -12,7 +13,9 @@ #include #include +#include "base/containers/fixed_flat_set.h" #include "base/functional/bind.h" +#include "base/ranges/algorithm.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "content/public/browser/browser_thread.h" @@ -22,6 +25,170 @@ namespace brave_ads { namespace { +// A matcher for MAC addresses, to allows us to test against a mask of bytes. +class MacAddressInfoMatcher { + public: + template + constexpr MacAddressInfoMatcher(Args... args) // NOLINT(runtime/explicit) + : address_{static_cast(args)...}, size_(sizeof...(args)) { + base::ranges::fill(base::span(address_).split_at(size_).second, 0u); + static_assert(sizeof...(args) <= 6u, "Invalid MAC address size"); + } + + constexpr MacAddressInfoMatcher(const MacAddressInfoMatcher&) = default; + constexpr MacAddressInfoMatcher& operator=( + const MacAddressInfoMatcher& other) { + base::ranges::copy(other.address_, address_.begin()); + size_ = other.size_; + return *this; + } + + constexpr base::span mask() const { + return base::span(address_).first(size_); + } + + private: + // The MAC address mask. + std::array address_; + + // The size of the partial mask. + size_t size_; +}; + +// A comparator for MAC addresses, so it can be used with a flat_set. +struct MacAddressInfoComparator { + using is_transparent = void; + constexpr bool operator()(const MacAddressInfoMatcher& a, + const MacAddressInfoMatcher& b) const { + return base::ranges::lexicographical_compare(a.mask(), b.mask()); + } + constexpr bool operator()(const MacAddressInfoMatcher& matcher, + base::span address) const { + return base::ranges::lexicographical_compare( + matcher.mask(), + address.first(std::min(matcher.mask().size(), address.size()))); + } + constexpr bool operator()(base::span address, + const MacAddressInfoMatcher& matcher) const { + return base::ranges::lexicographical_compare( + address.first(std::min(matcher.mask().size(), address.size())), + matcher.mask()); + } +}; + +constexpr auto kInvalidMacAddresses = + base::MakeFixedFlatSet( + { + // Empty address + {0x00, 0x00, 0x00, 0x00, 0x00, 0x00}, + // VMware + {0x00, 0x50, 0x56}, + {0x00, 0x05, 0x69}, + {0x00, 0x0c, 0x29}, + {0x00, 0x1c, 0x14}, + // VirtualBox + {0x08, 0x00, 0x27}, + // PdaNet + {0x00, 0x26, 0x37, 0xbd, 0x39, 0x42}, + // Cisco AnyConnect VPN + {0x00, 0x05, 0x9a, 0x3c, 0x7a, 0x00}, + // Marvell sometimes uses this as a dummy address + {0x00, 0x11, 0x22, 0x33, 0x44, 0x55}, + // Apple uses this across machines for Bluetooth ethernet adapters. + {0x65, 0x90, 0x07, 0x42, 0xf1}, + // Juniper uses this for their Virtual Adapter, the other 4 bytes + // are + // reassigned at every boot. 00-ff-xx is not assigned to anyone. + {0x00, 0xff}, + // Generic Bluetooth device + {0x00, 0x15, 0x83, 0x3d, 0x0a, 0x57}, + // RAS Async Adapter + {0x20, 0x41, 0x53, 0x59, 0x4e, 0xff}, + // T-Mobile Wireless Ethernet + // Qualcomm USB ethernet adapter + {0x00, 0xa0, 0xc6, 0x00, 0x00, 0x00}, + // Windows VPN + {0x00, 0x53, 0x45, 0x00, 0x00, 0x00}, + // Bluetooth + {0x00, 0x1f, 0x81, 0x00, 0x08, 0x30}, + {0x00, 0x1b, 0x10, 0x00, 0x2a, 0xec}, + {0x00, 0x15, 0x83, 0x15, 0xa3, 0x10}, + {0x00, 0x15, 0x83, 0x07, 0xC6, 0x5A}, + {0x00, 0x1f, 0x81, 0x00, 0x02, 0x00}, + {0x00, 0x1f, 0x81, 0x00, 0x02, 0xdd}, + // Ceton TV tuner + {0x00, 0x22, 0x2c, 0xff, 0xff, 0xff}, + // Check Point VPN + {0x54, 0x55, 0x43, 0x44, 0x52, 0x09}, + {0x54, 0xEF, 0x14, 0x71, 0xE4, 0x0E}, + {0x54, 0xBA, 0xC6, 0xFF, 0x74, 0x10}, + // Cisco VPN + {0x00, 0x05, 0x9a, 0x3c, 0x78, 0x00}, + // Intel USB cell modem + {0x00, 0x1e, 0x10, 0x1f, 0x00, 0x01}, + // Microsoft tethering + {0x80, 0x00, 0x60, 0x0f, 0xe8, 0x00}, + // Nortel VPN + {0x44, 0x45, 0x53, 0x54, 0x42, 0x00}, + // AEP VPN + {0x00, 0x30, 0x70, 0x00, 0x00, 0x01}, + // Positive VPN + {0x00, 0x02, 0x03, 0x04, 0x05, 0x06}, + // Bluetooth + {0x00, 0x15, 0x83, 0x0B, 0x13, 0xC0}, + // Kerio Virtual Network Adapter + {0x44, 0x45, 0x53, 0x54, 0x4f, 0x53}, + // Sierra Wireless cell modems. + {0x00, 0xA0, 0xD5}, + // FRITZ!web DSL + {0x00, 0x04, 0x0E, 0xFF, 0xFF, 0xFF}, + // VirtualPC + {0x00, 0x00, 0x00, 0x00, 0x00, 0x01}, + // Bluetooth + {0x00, 0x1F, 0x81, 0x00, 0x01, 0x00}, + {0x00, 0x30, 0x91, 0x10, 0x00, 0x26}, + {0x00, 0x25, 0x00, 0x5A, 0xC3, 0xD0}, + {0x00, 0x15, 0x83, 0x0C, 0xBF, 0xEB}, + // Huawei cell modem + {0x58, 0x2C, 0x80, 0x13, 0x92, 0x63}, + // Fortinet VPN + {0x00, 0x09, 0x0F}, + // Realtek + {0x00, 0x00, 0x00, 0x00, 0x00, 0x30}, + // Other rare dupes. + {0x00, 0x11, 0xf5, 0x0d, 0x8a, 0xe8}, // Atheros + {0x00, 0x20, 0x07, 0x01, 0x16, 0x06}, // Atheros + {0x0d, 0x0b, 0x00, 0x00, 0xe0, 0x00}, // Atheros + {0x90, 0x4c, 0xe5, 0x0b, 0xc8, 0x8e}, // Atheros + {0x00, 0x1c, 0x23, 0x38, 0x49, 0xa4}, // Broadcom + {0x00, 0x12, 0x3f, 0x82, 0x7c, 0x32}, // Broadcom + {0x00, 0x11, 0x11, 0x32, 0xc3, 0x77}, // Broadcom + {0x00, 0x24, 0xd6, 0xae, 0x3e, 0x39}, // Microsoft + {0x00, 0x0f, 0xb0, 0x3a, 0xb4, 0x80}, // Realtek + {0x08, 0x10, 0x74, 0xa1, 0xda, 0x1b}, // Realtek + {0x00, 0x21, 0x9b, 0x2a, 0x0a, 0x9c}, // Realtek + }, + MacAddressInfoComparator()); + +// Ensure that invalid MAC addresses are not submasked. This is important as the +// binary search for the elements could fall past the bunch, if we had +// submasking groups, and then it wouldn't find the actual mask. +constexpr bool HasNoSubmaskingOnInvalidAddresses() { + if constexpr (kInvalidMacAddresses.size() < 2u) { + return true; + } + for (auto it = std::cbegin(kInvalidMacAddresses); + it != std::cend(kInvalidMacAddresses) - 1; ++it) { + auto next = std::next(it)->mask(); + if (it->mask() == next.first(std::min(it->mask().size(), next.size()))) { + return false; + } + } + return true; +} +static_assert(HasNoSubmaskingOnInvalidAddresses(), + "Invalid MAC addresses must not be submasked"); + std::optional ComputeHmacSha256(std::string_view key, std::string_view text) { crypto::HMAC hmac(crypto::HMAC::SHA256); @@ -46,128 +213,6 @@ void GetRawDeviceIdCallback(DeviceIdCallback callback, ComputeHmacSha256(raw_device_id, "FOOBAR").value_or(std::string())); } -bool IsValidMacAddressImpl(const void* bytes, size_t size) { - constexpr size_t kMacLength = 6; - - struct MacAddressInfo { - size_t count; - unsigned char address[kMacLength]; - }; - - constexpr size_t kOuiLength = 3; - - // VPN, virtualization, tethering, bluetooth, etc. - static constexpr MacAddressInfo kInvalidMacAddresses[] = { - // Empty address - {kMacLength, {0, 0, 0, 0, 0, 0}}, - // VMware - {kOuiLength, {0x00, 0x50, 0x56}}, - {kOuiLength, {0x00, 0x05, 0x69}}, - {kOuiLength, {0x00, 0x0c, 0x29}}, - {kOuiLength, {0x00, 0x1c, 0x14}}, - // VirtualBox - {kOuiLength, {0x08, 0x00, 0x27}}, - // PdaNet - {kMacLength, {0x00, 0x26, 0x37, 0xbd, 0x39, 0x42}}, - // Cisco AnyConnect VPN - {kMacLength, {0x00, 0x05, 0x9a, 0x3c, 0x7a, 0x00}}, - // Marvell sometimes uses this as a dummy address - {kMacLength, {0x00, 0x11, 0x22, 0x33, 0x44, 0x55}}, - // Apple uses this across machines for Bluetooth ethernet adapters. - {kMacLength - 1, {0x65, 0x90, 0x07, 0x42, 0xf1}}, - // Juniper uses this for their Virtual Adapter, the other 4 bytes are - // reassigned at every boot. 00-ff-xx is not assigned to anyone. - {2, {0x00, 0xff}}, - // T-Mobile Wireless Ethernet - {kMacLength, {0x00, 0xa0, 0xc6, 0x00, 0x00, 0x00}}, - // Generic Bluetooth device - {kMacLength, {0x00, 0x15, 0x83, 0x3d, 0x0a, 0x57}}, - // RAS Async Adapter - {kMacLength, {0x20, 0x41, 0x53, 0x59, 0x4e, 0xff}}, - // Qualcomm USB ethernet adapter - {kMacLength, {0x00, 0xa0, 0xc6, 0x00, 0x00, 0x00}}, - // Windows VPN - {kMacLength, {0x00, 0x53, 0x45, 0x00, 0x00, 0x00}}, - // Bluetooth - {kMacLength, {0x00, 0x1f, 0x81, 0x00, 0x08, 0x30}}, - {kMacLength, {0x00, 0x1b, 0x10, 0x00, 0x2a, 0xec}}, - {kMacLength, {0x00, 0x15, 0x83, 0x15, 0xa3, 0x10}}, - {kMacLength, {0x00, 0x15, 0x83, 0x07, 0xC6, 0x5A}}, - {kMacLength, {0x00, 0x1f, 0x81, 0x00, 0x02, 0x00}}, - {kMacLength, {0x00, 0x1f, 0x81, 0x00, 0x02, 0xdd}}, - // Ceton TV tuner - {kMacLength, {0x00, 0x22, 0x2c, 0xff, 0xff, 0xff}}, - // Check Point VPN - {kMacLength, {0x54, 0x55, 0x43, 0x44, 0x52, 0x09}}, - {kMacLength, {0x54, 0xEF, 0x14, 0x71, 0xE4, 0x0E}}, - {kMacLength, {0x54, 0xBA, 0xC6, 0xFF, 0x74, 0x10}}, - // Cisco VPN - {kMacLength, {0x00, 0x05, 0x9a, 0x3c, 0x7a, 0x00}}, - // Cisco VPN - {kMacLength, {0x00, 0x05, 0x9a, 0x3c, 0x78, 0x00}}, - // Intel USB cell modem - {kMacLength, {0x00, 0x1e, 0x10, 0x1f, 0x00, 0x01}}, - // Microsoft tethering - {kMacLength, {0x80, 0x00, 0x60, 0x0f, 0xe8, 0x00}}, - // Nortel VPN - {kMacLength, {0x44, 0x45, 0x53, 0x54, 0x42, 0x00}}, - // AEP VPN - {kMacLength, {0x00, 0x30, 0x70, 0x00, 0x00, 0x01}}, - // Positive VPN - {kMacLength, {0x00, 0x02, 0x03, 0x04, 0x05, 0x06}}, - // Bluetooth - {kMacLength, {0x00, 0x15, 0x83, 0x0B, 0x13, 0xC0}}, - // Kerio Virtual Network Adapter - {kMacLength, {0x44, 0x45, 0x53, 0x54, 0x4f, 0x53}}, - // Sierra Wireless cell modems. - {kOuiLength, {0x00, 0xA0, 0xD5}}, - // FRITZ!web DSL - {kMacLength, {0x00, 0x04, 0x0E, 0xFF, 0xFF, 0xFF}}, - // VirtualPC - {kMacLength, {0x00, 0x00, 0x00, 0x00, 0x00, 0x01}}, - // Bluetooth - {kMacLength, {0x00, 0x1F, 0x81, 0x00, 0x01, 0x00}}, - {kMacLength, {0x00, 0x30, 0x91, 0x10, 0x00, 0x26}}, - {kMacLength, {0x00, 0x25, 0x00, 0x5A, 0xC3, 0xD0}}, - {kMacLength, {0x00, 0x15, 0x83, 0x0C, 0xBF, 0xEB}}, - // Huawei cell modem - {kMacLength, {0x58, 0x2C, 0x80, 0x13, 0x92, 0x63}}, - // Fortinet VPN - {kOuiLength, {0x00, 0x09, 0x0F}}, - // Realtek - {kMacLength, {0x00, 0x00, 0x00, 0x00, 0x00, 0x30}}, - // Other rare dupes. - {kMacLength, {0x00, 0x11, 0xf5, 0x0d, 0x8a, 0xe8}}, // Atheros - {kMacLength, {0x00, 0x20, 0x07, 0x01, 0x16, 0x06}}, // Atheros - {kMacLength, {0x0d, 0x0b, 0x00, 0x00, 0xe0, 0x00}}, // Atheros - {kMacLength, {0x90, 0x4c, 0xe5, 0x0b, 0xc8, 0x8e}}, // Atheros - {kMacLength, {0x00, 0x1c, 0x23, 0x38, 0x49, 0xa4}}, // Broadcom - {kMacLength, {0x00, 0x12, 0x3f, 0x82, 0x7c, 0x32}}, // Broadcom - {kMacLength, {0x00, 0x11, 0x11, 0x32, 0xc3, 0x77}}, // Broadcom - {kMacLength, {0x00, 0x24, 0xd6, 0xae, 0x3e, 0x39}}, // Microsoft - {kMacLength, {0x00, 0x0f, 0xb0, 0x3a, 0xb4, 0x80}}, // Realtek - {kMacLength, {0x08, 0x10, 0x74, 0xa1, 0xda, 0x1b}}, // Realtek - {kMacLength, {0x00, 0x21, 0x9b, 0x2a, 0x0a, 0x9c}}, // Realtek - }; - - if (size != kMacLength) { - return false; - } - - if (static_cast(bytes)[0] & 0x02) { - // Locally administered. - return false; - } - - for (const auto& mac_address : kInvalidMacAddresses) { - if (memcmp(mac_address.address, bytes, mac_address.count) == 0) { - return false; - } - } - - return true; -} - } // namespace void DeviceIdImpl::GetDeviceId(DeviceIdCallback callback) { @@ -179,8 +224,14 @@ void DeviceIdImpl::GetDeviceId(DeviceIdCallback callback) { } // static -bool DeviceIdImpl::IsValidMacAddress(const void* bytes, size_t size) { - return IsValidMacAddressImpl(bytes, size); +bool DeviceIdImpl::IsValidMacAddress(base::span bytes) { + constexpr size_t kMacLength = 6; + if (bytes.size() != kMacLength || bytes.front() & 0x02) { + // Locally administered. + return false; + } + + return kInvalidMacAddresses.find(bytes) == kInvalidMacAddresses.end(); } } // namespace brave_ads diff --git a/browser/brave_ads/device_id/device_id_impl.h b/browser/brave_ads/device_id/device_id_impl.h index e287165c72a5..f82e49161441 100644 --- a/browser/brave_ads/device_id/device_id_impl.h +++ b/browser/brave_ads/device_id/device_id_impl.h @@ -6,6 +6,7 @@ #ifndef BRAVE_BROWSER_BRAVE_ADS_DEVICE_ID_DEVICE_ID_IMPL_H_ #define BRAVE_BROWSER_BRAVE_ADS_DEVICE_ID_DEVICE_ID_IMPL_H_ +#include "base/containers/span.h" #include "brave/components/brave_ads/browser/device_id/device_id.h" namespace brave_ads { @@ -14,15 +15,15 @@ class DeviceIdImpl : public DeviceId { public: void GetDeviceId(DeviceIdCallback callback) override; - private: - // Platform specific implementation of "raw" device id retrieval. - static void GetRawDeviceId(DeviceIdCallback callback); - // On some platforms, part of the machine ID is the MAC address. This function // is shared across platforms to filter out MAC addresses that have been // identified as invalid, i.e. not unique. For example, some VM hosts assign a // new MAC address at each reboot. - static bool IsValidMacAddress(const void* bytes, size_t size); + static bool IsValidMacAddress(base::span bytes); + + private: + // Platform specific implementation of "raw" device id retrieval. + static void GetRawDeviceId(DeviceIdCallback callback); }; } // namespace brave_ads diff --git a/browser/brave_ads/device_id/device_id_impl_linux.cc b/browser/brave_ads/device_id/device_id_impl_linux.cc index c9a48809271b..36336080f2db 100644 --- a/browser/brave_ads/device_id/device_id_impl_linux.cc +++ b/browser/brave_ads/device_id/device_id_impl_linux.cc @@ -34,7 +34,7 @@ namespace brave_ads { using IsValidMacAddressCallback = - base::RepeatingCallback; + base::RepeatingCallback bytes)>; using DiskMap = std::map; @@ -99,7 +99,6 @@ class MacAddressProcessor { size_t prefixes_count) { bool keep_going = true; - constexpr int kMacLength = 6; struct ifreq ifinfo; memset(&ifinfo, 0, sizeof(ifinfo)); @@ -113,9 +112,10 @@ class MacAddressProcessor { return keep_going; } - const char* mac_address = - static_cast(ifinfo.ifr_hwaddr.sa_data); - if (!is_valid_mac_address_callback_.Run(mac_address, kMacLength)) { + constexpr size_t kMacLength = 6u; + auto mac_address_bytes = + base::as_bytes(base::make_span(ifinfo.ifr_hwaddr.sa_data, kMacLength)); + if (!is_valid_mac_address_callback_.Run(mac_address_bytes)) { return keep_going; } @@ -125,7 +125,7 @@ class MacAddressProcessor { keep_going = false; - mac_address_ = base::ToLowerASCII(base::HexEncode(mac_address, kMacLength)); + mac_address_ = base::ToLowerASCII(base::HexEncode(mac_address_bytes)); return keep_going; } diff --git a/browser/brave_ads/device_id/device_id_impl_mac.cc b/browser/brave_ads/device_id/device_id_impl_mac.cc index c8559adb8798..4b3577faecdf 100644 --- a/browser/brave_ads/device_id/device_id_impl_mac.cc +++ b/browser/brave_ads/device_id/device_id_impl_mac.cc @@ -24,6 +24,7 @@ #include "base/location.h" #include "base/mac/mac_util.h" #include "base/mac/scoped_ioobject.h" +#include "base/numerics/safe_conversions.h" #include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" @@ -37,7 +38,7 @@ namespace brave_ads { using IsValidMacAddressCallback = - base::RepeatingCallback; + base::RepeatingCallback bytes)>; namespace { @@ -137,14 +138,14 @@ class MacAddressProcessor { return keep_going; } - const UInt8* mac_address = CFDataGetBytePtr(mac_address_data.get()); - const size_t mac_address_size = CFDataGetLength(mac_address_data.get()); - if (!is_valid_mac_address_callback_.Run(mac_address, mac_address_size)) { + auto mac_address_bytes = base::as_bytes(base::make_span( + CFDataGetBytePtr(mac_address_data.get()), + base::checked_cast(CFDataGetLength(mac_address_data.get())))); + if (!is_valid_mac_address_callback_.Run(mac_address_bytes)) { return keep_going; } - mac_address_ = - base::ToLowerASCII(base::HexEncode(mac_address, mac_address_size)); + mac_address_ = base::ToLowerASCII(base::HexEncode(mac_address_bytes)); base::apple::ScopedCFTypeRef provider_class_string( static_cast(IORegistryEntryCreateCFProperty( diff --git a/browser/brave_ads/device_id/device_id_impl_unittest.cc b/browser/brave_ads/device_id/device_id_impl_unittest.cc new file mode 100644 index 000000000000..beec81e2b1bf --- /dev/null +++ b/browser/brave_ads/device_id/device_id_impl_unittest.cc @@ -0,0 +1,149 @@ +/* Copyright (c) 2024 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/brave_ads/device_id/device_id_impl.h" + +#include + +#include "base/ranges/algorithm.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace brave_ads { + +namespace { + +struct DisallwedMask { + template + constexpr DisallwedMask(Args... args) // NOLINT(runtime/explicit) + : address{static_cast(args)...}, size(sizeof...(args)) {} + + base::span as_span() const { + return base::span(address).first(size); + } + + std::array address; + size_t size; +}; + +void TestDisallowedRange(DisallwedMask mask) { + std::array address; + + // test the lower bound of the match + base::ranges::fill(address, 0u); + base::as_writable_byte_span(address).first(mask.size).copy_from( + mask.as_span()); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress(address)); + + // test the upper bound of the match + base::ranges::fill( + base::as_writable_byte_span(address).split_at(mask.size).second, 0xffu); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress(address)); + + // test accepted just outside the upper bound + base::as_writable_byte_span(address).first(mask.size).copy_from( + mask.as_span()); + base::as_writable_byte_span(address).first(mask.size).last<1>()[0] += 0x01; + base::ranges::fill( + base::as_writable_byte_span(address).split_at(mask.size).second, 0x00u); + EXPECT_TRUE(DeviceIdImpl::IsValidMacAddress(address)); + + // test accepted just outside the lower bound + base::as_writable_byte_span(address).first(mask.size).copy_from( + mask.as_span()); + base::as_writable_byte_span(address).first(mask.size).last<1>()[0] -= 0x01; + base::ranges::fill( + base::as_writable_byte_span(address).split_at(mask.size).second, 0xffu); + EXPECT_TRUE(DeviceIdImpl::IsValidMacAddress(address)); +} + +} // namespace + +TEST(DeviceIdImplTest, InvalidMacAddresses) { + // VMware + TestDisallowedRange({0x00, 0x50, 0x56}); + TestDisallowedRange({0x00, 0x05, 0x69}); + TestDisallowedRange({0x00, 0x0c, 0x29}); + TestDisallowedRange({0x00, 0x1c, 0x14}); + // VirtualBox + TestDisallowedRange({0x08, 0x00, 0x27}); + // Sierra Wireless cell modems. + TestDisallowedRange({0x00, 0xA0, 0xD5}); + // Fortinet VPN + TestDisallowedRange({0x00, 0x09, 0x0F}); + + // Juniper + EXPECT_TRUE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0xfe, 0xff, 0xff, 0xff, 0xff}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0xff, 0x00, 0x00, 0x00, 0x00}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0xff, 0xff, 0xff, 0xff, 0xff}))); + EXPECT_TRUE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x01, 0x00, 0x00, 0x00, 0x00, 0x00}))); + + // T-Mobile Wireless Ethernet + // Qualcomm USB ethernet adapter + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0xa0, 0xc6, 0x00, 0x00, 0x00}))); + // Windows VPN + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x53, 0x45, 0x00, 0x00, 0x00}))); + // Bluetooth + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x1f, 0x81, 0x00, 0x08, 0x30}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x1b, 0x10, 0x00, 0x2a, 0xec}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x15, 0x83, 0x15, 0xa3, 0x10}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x15, 0x83, 0x07, 0xC6, 0x5A}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x1f, 0x81, 0x00, 0x02, 0x00}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x1f, 0x81, 0x00, 0x02, 0xdd}))); + // Ceton TV tuner + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x22, 0x2c, 0xff, 0xff, 0xff}))); + // Check Point VPN + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x54, 0x55, 0x43, 0x44, 0x52, 0x09}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x54, 0xEF, 0x14, 0x71, 0xE4, 0x0E}))); + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x54, 0xBA, 0xC6, 0xFF, 0x74, 0x10}))); + // Cisco VPN + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x05, 0x9a, 0x3c, 0x78, 0x00}))); + // Intel USB cell modem + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x1e, 0x10, 0x1f, 0x00, 0x01}))); + // Microsoft tethering + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x80, 0x00, 0x60, 0x0f, 0xe8, 0x00}))); + // Nortel VPN + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x44, 0x45, 0x53, 0x54, 0x42, 0x00}))); + // AEP VPN + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x30, 0x70, 0x00, 0x00, 0x01}))); + // Positive VPN + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x02, 0x03, 0x04, 0x05, 0x06}))); + // Bluetooth + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x15, 0x83, 0x0B, 0x13, 0xC0}))); + + // unknown by last byte form the last test + EXPECT_TRUE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x00, 0x15, 0x83, 0x0B, 0x13, 0xC1}))); + + // Fails because range is too short + EXPECT_FALSE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x01, 0x14, 0x85}))); + EXPECT_TRUE(DeviceIdImpl::IsValidMacAddress( + std::to_array({0x01, 0x14, 0x85, 0x01, 0x14, 0x85}))); +} + +} // namespace brave_ads diff --git a/browser/brave_ads/device_id/device_id_impl_win.cc b/browser/brave_ads/device_id/device_id_impl_win.cc index 181fd2bc6142..10c4792d3dd7 100644 --- a/browser/brave_ads/device_id/device_id_impl_win.cc +++ b/browser/brave_ads/device_id/device_id_impl_win.cc @@ -42,7 +42,7 @@ namespace brave_ads { namespace { using IsValidMacAddressCallback = - base::RepeatingCallback; + base::RepeatingCallback bytes)>; class MacAddressProcessor { public: @@ -82,10 +82,13 @@ class MacAddressProcessor { if (index >= found_index_ || size == 0) return; - if (!is_valid_mac_address_callback_.Run(bytes, size)) + auto mac_address_bytes = + base::span(static_cast(bytes), size); + if (!is_valid_mac_address_callback_.Run(mac_address_bytes)) { return; + } - mac_address_ = base::ToLowerASCII(base::HexEncode(bytes, size)); + mac_address_ = base::ToLowerASCII(base::HexEncode(mac_address_bytes)); found_index_ = index; } diff --git a/browser/default_protocol_handler_utils_win.cc b/browser/default_protocol_handler_utils_win.cc index fd8560ec35a3..d7c173336ce2 100644 --- a/browser/default_protocol_handler_utils_win.cc +++ b/browser/default_protocol_handler_utils_win.cc @@ -6,20 +6,25 @@ #include "brave/browser/default_protocol_handler_utils_win.h" #include + #include #include #include #include +#include #include #include "base/base64.h" #include "base/containers/span.h" +#include "base/containers/span_reader.h" #include "base/files/file_path.h" #include "base/hash/md5.h" #include "base/logging.h" #include "base/notreached.h" +#include "base/numerics/byte_conversions.h" #include "base/path_service.h" +#include "base/strings/cstring_view.h" #include "base/strings/strcat.h" #include "base/strings/string_util_win.h" #include "base/strings/stringprintf.h" @@ -45,57 +50,57 @@ constexpr wchar_t kUserChoiceKey[] = L"UserChoice"; constexpr wchar_t kProgIDKey[] = L"ProgID"; constexpr wchar_t kHashKey[] = L"Hash"; -inline DWORD WordSwap(DWORD v) { +inline uint32_t WordSwap(uint32_t v) { return (v >> 16) | (v << 16); } -std::wstring HashString(std::wstring_view input_string) { - auto* input_bytes = - reinterpret_cast(input_string.data()); - const size_t input_byte_count = (input_string.length() + 1) * sizeof(wchar_t); - - constexpr size_t kDWordsPerBlock = 2; - constexpr size_t kBlockSize = sizeof(DWORD) * kDWordsPerBlock; - // Incomplete blocks are ignored. - const int block_count = input_byte_count / kBlockSize; - - if (block_count == 0) { +std::wstring HashString(base::wcstring_view input) { + if (input.size() < 8u) { + // It seems this algo doesn't consider anything on the modulo of blocks, so + // there's nothing for us to do with small strings. return std::wstring(); } + auto bytes = base::as_bytes(base::make_span(input.data(), input.size() + 1)); // Compute an MD5 hash. md5[0] and md5[1] will be used as constant multipliers // in the scramble below. base::MD5Digest digest; - base::MD5Sum(base::span(input_bytes, input_byte_count), &digest); - auto* md5 = reinterpret_cast(digest.a.data()); + base::MD5Sum(bytes, &digest); + auto md5 = base::as_byte_span(digest.a).first<8u>(); + // The following loop effectively computes two checksums, scrambled like a // hash after every DWORD is added. - - // Constant multipliers for the scramble, one set for each DWORD in a block. - const DWORD c0s[kDWordsPerBlock][5] = { - {md5[0] | 1, 0xCF98B111uL, 0x87085B9FuL, 0x12CEB96DuL, 0x257E1D83uL}, - {md5[1] | 1, 0xA27416F5uL, 0xD38396FFuL, 0x7C932B89uL, 0xBFA49F69uL}}; - const DWORD c1s[kDWordsPerBlock][5] = { - {md5[0] | 1, 0xEF0569FBuL, 0x689B6B9FuL, 0x79F8A395uL, 0xC3EFEA97uL}, - {md5[1] | 1, 0xC31713DBuL, 0xDDCD1F0FuL, 0x59C3AF2DuL, 0x35BD1EC9uL}}; + struct MagicBlock { + std::pair, std::array> line; + }; + + auto magic_numbers_table = std::to_array( + {{{{base::numerics::U32FromNativeEndian(md5.first<4u>()) | 1, + 0xCF98B111uL, 0x87085B9FuL, 0x12CEB96DuL, 0x257E1D83uL}, + {base::numerics::U32FromNativeEndian(md5.first<4u>()) | 1, + 0xEF0569FBuL, 0x689B6B9FuL, 0x79F8A395uL, 0xC3EFEA97uL}}}, + {{{base::numerics::U32FromNativeEndian(md5.last<4u>()) | 1, 0xA27416F5uL, + 0xD38396FFuL, 0x7C932B89uL, 0xBFA49F69uL}, + {base::numerics::U32FromNativeEndian(md5.last<4u>()) | 1, 0xC31713DBuL, + 0xDDCD1F0FuL, 0x59C3AF2DuL, 0x35BD1EC9uL}}}}); // The checksums. - DWORD h0 = 0; - DWORD h1 = 0; - // Accumulated total of the checksum after each DWORD. - DWORD h0_acc = 0; - DWORD h1_acc = 0; - - for (int i = 0; i < block_count; ++i) { - for (size_t j = 0; j < kDWordsPerBlock; ++j) { - const DWORD* c0 = c0s[j]; - const DWORD* c1 = c1s[j]; - - DWORD input; - memcpy(&input, &input_bytes[(i * kDWordsPerBlock + j) * sizeof(DWORD)], - sizeof(DWORD)); - - h0 += input; + uint32_t h0 = 0; + uint32_t h1 = 0; + // Accumulated total of the checksum after each uint32_t. + uint32_t h0_acc = 0; + uint32_t h1_acc = 0; + + auto reader = base::SpanReader(bytes); + while (reader.remaining() >= 8u) { + // We clearly don't care about anything under 8u bytes as the mozilla + // implementation also doesn't seem to care. + + for (const auto& magic_block : magic_numbers_table) { + const auto& [c0, c1] = magic_block.line; + uint32_t ui32 = base::numerics::U32FromNativeEndian(*reader.Read<4u>()); + + h0 += ui32; // Scramble 0 h0 *= c0[0]; h0 = WordSwap(h0) * c0[1]; @@ -104,7 +109,7 @@ std::wstring HashString(std::wstring_view input_string) { h0 = WordSwap(h0) * c0[4]; h0_acc += h0; - h1 += input; + h1 += ui32; // Scramble 1 h1 = WordSwap(h1) * c1[1] + h1 * c1[0]; h1 = (h1 >> 16) * c1[2] + h1 * c1[3]; @@ -113,7 +118,7 @@ std::wstring HashString(std::wstring_view input_string) { } } - DWORD hash[2] = {h0 ^ h1, h0_acc ^ h1_acc}; + uint32_t hash[2] = {h0 ^ h1, h0_acc ^ h1_acc}; return base::UTF8ToWide( base::Base64Encode(base::as_bytes(base::make_span(hash)))); } diff --git a/components/brave_shields/content/browser/brave_farbling_service.cc b/components/brave_shields/content/browser/brave_farbling_service.cc index 020cd3ea133e..6af841bd2c77 100644 --- a/components/brave_shields/content/browser/brave_farbling_service.cc +++ b/components/brave_shields/content/browser/brave_farbling_service.cc @@ -8,6 +8,7 @@ #include #include "base/feature_list.h" +#include "base/numerics/byte_conversions.h" #include "base/rand_util.h" #include "brave/components/brave_shields/core/common/features.h" #include "brave/components/brave_shields/core/common/pref_names.h" @@ -45,11 +46,11 @@ bool BraveFarblingService::MakePseudoRandomGeneratorForURL(const GURL& url, uint8_t domain_key[32]; uint64_t session_key = session_token(); crypto::HMAC h(crypto::HMAC::SHA256); - CHECK(h.Init(reinterpret_cast(&session_key), - sizeof session_key)); - CHECK(h.Sign(domain, domain_key, sizeof domain_key)); - uint64_t seed = *reinterpret_cast(domain_key); - *prng = FarblingPRNG(seed); + CHECK(h.Init(base::byte_span_from_ref(session_key))); + CHECK(h.Sign(base::as_byte_span(domain), + base::as_writable_byte_span(domain_key))); + *prng = FarblingPRNG( + base::U64FromNativeEndian(base::as_byte_span(domain_key).first<8>())); return true; } diff --git a/test/BUILD.gn b/test/BUILD.gn index c3ea3f2e27db..d66a6783a620 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -157,6 +157,7 @@ test("brave_unit_tests") { "//brave/browser", "//brave/browser:unit_tests", "//brave/browser/brave_ads", + "//brave/browser/brave_ads/device_id:unit_tests", "//brave/browser/brave_wallet", "//brave/browser/brave_wallet:unit_tests", "//brave/browser/content_settings:unit_tests",