From 935ab42b1ecb3167f05439566952f330500cbb3d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 20 Mar 2026 21:55:06 -1000 Subject: [PATCH] [esp32] Validate eFuse MAC reads and reject garbage MACs On some ESP32 boards (especially cheap clones), the eFuse custom MAC area contains random garbage that passes the existing all-zeros/all-ones validation. Additionally, esp_efuse_mac_get_default() can fail with CRC errors, but the return value was being ignored, causing garbage MAC addresses to be advertised via mDNS. This caused Home Assistant to report false "MAC address changed" device conflicts on every boot. Two fixes: - Check return values from eFuse MAC read functions and add a fallback chain: custom MAC -> default MAC -> raw eFuse bytes -> zeroed MAC. - Reject multicast MACs (bit 0 of first byte set) in mac_address_is_valid() since device MACs must always be unicast. Closes https://github.com/esphome/esphome/issues/14501 --- esphome/components/esp32/helpers.cpp | 46 ++++++++++++++++++++++------ esphome/core/helpers.cpp | 11 ++++++- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/esphome/components/esp32/helpers.cpp b/esphome/components/esp32/helpers.cpp index 654a35b473..afcec8bfc7 100644 --- a/esphome/components/esp32/helpers.cpp +++ b/esphome/components/esp32/helpers.cpp @@ -9,11 +9,14 @@ #include #include +#include "esphome/core/log.h" #include "esp_random.h" #include "esp_system.h" namespace esphome { +static const char *const TAG = "esp32"; + bool random_bytes(uint8_t *data, size_t len) { esp_fill_random(data, len); return true; @@ -63,22 +66,43 @@ LwIPLock::~LwIPLock() { #endif } +/// Read MAC and validate both the return code and content. +static bool read_valid_mac(uint8_t *mac, esp_err_t err) { return err == ESP_OK && mac_address_is_valid(mac); } + +static constexpr size_t MAC_ADDRESS_SIZE_BITS = MAC_ADDRESS_SIZE * 8; // 48 bits + void get_mac_address_raw(uint8_t *mac) { // NOLINT(readability-non-const-parameter) #if defined(CONFIG_SOC_IEEE802154_SUPPORTED) // When CONFIG_SOC_IEEE802154_SUPPORTED is defined, esp_efuse_mac_get_default // returns the 802.15.4 EUI-64 address, so we read directly from eFuse instead. - if (has_custom_mac_address()) { - esp_efuse_read_field_blob(ESP_EFUSE_MAC_CUSTOM, mac, 48); - } else { - esp_efuse_read_field_blob(ESP_EFUSE_MAC_FACTORY, mac, 48); + // Both paths already read raw eFuse bytes, so there is no CRC-bypass fallback + // (unlike the non-IEEE802154 path where esp_efuse_mac_get_default does CRC checks). + if (has_custom_mac_address() && + read_valid_mac(mac, esp_efuse_read_field_blob(ESP_EFUSE_MAC_CUSTOM, mac, MAC_ADDRESS_SIZE_BITS))) { + return; + } + if (read_valid_mac(mac, esp_efuse_read_field_blob(ESP_EFUSE_MAC_FACTORY, mac, MAC_ADDRESS_SIZE_BITS))) { + return; } #else - if (has_custom_mac_address()) { - esp_efuse_mac_get_custom(mac); - } else { - esp_efuse_mac_get_default(mac); + if (has_custom_mac_address() && read_valid_mac(mac, esp_efuse_mac_get_custom(mac))) { + return; + } + if (read_valid_mac(mac, esp_efuse_mac_get_default(mac))) { + return; + } + // Default MAC read failed (e.g., eFuse CRC error) - try reading raw eFuse bytes + // directly, bypassing CRC validation. A MAC that passes mac_address_is_valid() + // (non-zero, non-broadcast, unicast) is almost certainly the real factory MAC + // with a corrupted CRC byte, which is far better than returning garbage or zeros. + if (read_valid_mac(mac, esp_efuse_read_field_blob(ESP_EFUSE_MAC_FACTORY, mac, MAC_ADDRESS_SIZE_BITS))) { + ESP_LOGW(TAG, "eFuse MAC CRC failed but raw bytes appear valid - using raw eFuse MAC"); + return; } #endif + // All methods failed - zero the MAC rather than returning garbage + ESP_LOGE(TAG, "Failed to read a valid MAC address from eFuse"); + memset(mac, 0, MAC_ADDRESS_SIZE); } void set_mac_address(uint8_t *mac) { esp_base_mac_addr_set(mac); } @@ -88,9 +112,11 @@ bool has_custom_mac_address() { uint8_t mac[6]; // do not use 'esp_efuse_mac_get_custom(mac)' because it drops an error in the logs whenever it fails #ifndef USE_ESP32_VARIANT_ESP32 - return (esp_efuse_read_field_blob(ESP_EFUSE_USER_DATA_MAC_CUSTOM, mac, 48) == ESP_OK) && mac_address_is_valid(mac); + return (esp_efuse_read_field_blob(ESP_EFUSE_USER_DATA_MAC_CUSTOM, mac, MAC_ADDRESS_SIZE_BITS) == ESP_OK) && + mac_address_is_valid(mac); #else - return (esp_efuse_read_field_blob(ESP_EFUSE_MAC_CUSTOM, mac, 48) == ESP_OK) && mac_address_is_valid(mac); + return (esp_efuse_read_field_blob(ESP_EFUSE_MAC_CUSTOM, mac, MAC_ADDRESS_SIZE_BITS) == ESP_OK) && + mac_address_is_valid(mac); #endif #else return false; diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index ee99c54196..1732fc72e8 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -863,7 +863,16 @@ bool mac_address_is_valid(const uint8_t *mac) { is_all_ones = false; } } - return !(is_all_zeros || is_all_ones); + if (is_all_zeros || is_all_ones) { + return false; + } + // Reject multicast MACs (bit 0 of first byte set) - device MACs must be unicast. + // This catches garbage data from corrupted eFuse custom MAC areas, which often + // has random values that would otherwise pass the all-zeros/all-ones check. + if (mac[0] & 0x01) { + return false; + } + return true; } void IRAM_ATTR HOT delay_microseconds_safe(uint32_t us) {