Use device name stored in the wendy_conf partition#19
Conversation
AI Security Review
wendy_conf flash partition) that is propagated into BLE advertising, mDNS hostname, and mDNS TXT records. The change is architecturally reasonable and the code is generally careful, but several security concerns exist: unvalidated external data from flash is used directly in logging, mDNS advertisement fields, and BLE advertising without sanitisation; a potential null-pointer dereference exists when BLE provisioning is disabled but the name is used; strdup-allocated memory has a use-after-free risk window; and the mDNS hostname sanitiser silently drops characters rather than rejecting or truncating the input, which could allow subtle hostname confusion. There are no hardcoded secrets, no cryptographic issues, and no PCI/HIPAA-scope data touched. SOC 2 CC7 (logging of unsanitised PII/data), ISO 27001 A.8 (input validation, secure coding), and NIST SI-10 (information input validation) are the primary controls at risk. |
| Severity | Standards | File | Line(s) | Title |
|---|---|---|---|---|
| HIGH | ISO27001-A.8, NIST-SI-10 | main/wendy_main.c |
~521, ~585 | Unsanitised flash-sourced device name used directly in log and BLE/WiFi APIs |
| HIGH | ISO27001-A.8, NIST-SI-10 | components/wendy_server/src/wendy_server.c |
~183–194 | Unsanitised device name injected into mDNS TXT record name field |
| MEDIUM | ISO27001-A.8, NIST-SI-10 | main/wendy_main.c |
~585 | Potential null-pointer dereference when wendy_ble_prov_get_device_name() returns NULL |
| MEDIUM | ISO27001-A.8, NIST-SI-10 | components/wendy_server/src/wendy_server.c |
~295–297 | strdup result not checked for NULL; use-after-free race on _device_name |
| MEDIUM | ISO27001-A.8, NIST-SI-10 | components/wendy_wifi/src/wendy_wifi.c |
~266 | Device name accepted into s_device_name without validation before mDNS hostname construction |
| LOW | SOC2-CC7, ISO27001-A.12 | main/wendy_main.c |
~521 | Device name (potentially PII) logged unconditionally at INFO level |
| LOW | ISO27001-A.8 | components/wendy_conf/src/wendy_conf.c |
~191 | Integer underflow possible in wendy_conf_copy_span when dest_size == 0 |
| INFORMATIONAL | ISO27001-A.8 | components/wendy_ble_prov/src/wendy_ble_prov.c |
~305–311 | MAC suffix uses only 2 bytes; low entropy identifier |
Detailed Findings
FINDING 1 — HIGH | ISO27001-A.8, NIST-SI-10
Unsanitised flash-sourced device name used directly in log and BLE/WiFi APIs
Description:
The device name is read from flash via wendy_conf_copy_span, which performs no character-set validation. The raw value is then passed directly to wendy_ble_prov_init, wendy_wifi_init, and ESP_LOGI. Flash contents can be manipulated by a physical attacker with access to the device (e.g. via JTAG, SPI tap, or a compromised provisioning tool). An adversarially crafted name could contain:
- Null bytes mid-string (bypassing
strlcpylength checks and splitting perceived identifiers) - Non-printable or control characters that corrupt logs, BLE AD structures, or mDNS packets
- Characters that are valid in the NimBLE GAP name field but illegal in DNS labels (RFC 1123), leading to mDNS resolution failures or hostname confusion
- Excessively long names that stress downstream fixed buffers (mitigated by
strlcpy, but the root cause remains)
Relevant snippet:
// main/wendy_main.c
wendy_conf_copy_span(device_name, sizeof(device_name), wendy_conf_get_device_name());
if (device_name[0]) {
ESP_LOGI(TAG, "device name: %s", device_name);
}
// ...
err = wendy_ble_prov_init(device_name[0] ? device_name : NULL, &ble_prov_cbs);Remediation:
Introduce a validation function (e.g. wendy_validate_device_name) that enforces an allowlist of [a-zA-Z0-9-], a minimum length of 1, and a maximum length (e.g. 32 characters). Apply it immediately after wendy_conf_copy_span and before any use of the name. If validation fails, fall back to the MAC-derived default and log a warning (not the invalid name itself).
static bool validate_device_name(const char *name) {
if (!name || !name[0]) return false;
for (size_t i = 0; name[i]; i++) {
unsigned char c = (unsigned char)name[i];
if (!isalnum(c) && c != '-') return false;
if (i >= 32) return false;
}
return true;
}Controls violated: ISO 27001 A.8.28 (secure coding), NIST SP 800-53 SI-10 (Information Input Validation).
FINDING 2 — HIGH | ISO27001-A.8, NIST-SI-10
Unsanitised device name injected into mDNS TXT record name field
Description:
The device name — originating from flash without validation — is placed verbatim into an mDNS TXT record as the value of the name key, and also passed as the mDNS instance name (first argument to mdns_service_add). mDNS TXT record values are length-prefixed but displayed by browsers, mobile apps, and log aggregators. A name containing \n, \r, or other control characters could cause log injection or display corruption in mDNS browsers. More critically, the free(_device_name) immediately after mdns_service_add is unsafe if the mDNS stack retains the pointer rather than copying the string (see Finding 4).
Relevant snippet:
// components/wendy_server/src/wendy_server.c
mdns_txt_item_t txt_items[] = {
{
.key = "name",
.value = _device_name // ← raw flash-sourced value
},
...
};
esp_err_t mdns_err = mdns_service_add(_device_name, "_wendy-lite", "_tcp",
WENDY_SERVER_PORT, txt_items, sizeof(txt_items) / sizeof(txt_items[0]));
free(_device_name);
_device_name = NULL;Remediation:
- Validate the device name at ingestion (see Finding 1 remediation).
- Do not free
_device_nameimmediately aftermdns_service_adduntil it is confirmed the IDF mDNS implementation copies its string arguments. Reviewesp-idf/components/mdnssource; if it stores pointers, keep the buffer alive for the lifetime of the service. Use a module-level static buffer rather thanstrdup/free.
Controls violated: ISO 27001 A.8.28, NIST SI-10, NIST SC-5 (protection from DoS via malformed input).
FINDING 3 — MEDIUM | ISO27001-A.8, NIST-SI-10
Potential null-pointer dereference: wendy_ble_prov_get_device_name() may return NULL
Description:
When device_name[0] is false (no name in flash), the code waits for EVT_BLE_UP and then calls strlcpy(device_name, wendy_ble_prov_get_device_name(), sizeof(device_name)). The contract of wendy_ble_prov_get_device_name() is documented as returning NULL while the temp name is still in place. Although on_ble_up is called after s_temp_device_name is cleared (from build_device_name → prov_on_sync → callback), there is a subtle ordering dependency: if the sync path hits the ble_hs_util_ensure_addr fallback (returns non-zero), build_device_name returns early without clearing s_temp_device_name, so prov_on_sync calls on_ble_up with s_temp_device_name still true, and get_device_name() returns NULL. strlcpy with a NULL source is undefined behaviour on most libc implementations.
Relevant snippet:
// main/wendy_main.c
if (!device_name[0]) {
strlcpy(device_name, wendy_ble_prov_get_device_name(), sizeof(device_name));
// ↑ get_device_name() can return NULL here (fallback path in build_device_name)Remediation:
if (!device_name[0]) {
const char *ble_name = wendy_ble_prov_get_device_name();
if (ble_name) {
strlcpy(device_name, ble_name, sizeof(device_name));
} else {
snprintf(device_name, sizeof(device_name), "%s-0000",
CONFIG_WENDY_DEVICE_NAME_DEFAULT_PREFIX);
ESP_LOGW(TAG, "BLE name unavailable, using fallback: %s", device_name);
}
}Also, in build_device_name, the fallback path (rc != 0) should still clear s_temp_device_name so the NULL-return contract is consistent.
Controls violated: ISO 27001 A.8.28 (defensive programming / null-safety), NIST SI-16 (memory protection).
FINDING 4 — MEDIUM | ISO27001-A.8, NIST-SI-10
strdup result unchecked for NULL; potential use-after-free on _device_name
Description:
strdup can return NULL on allocation failure (especially on a constrained ESP32 with fragmented heap after NimBLE and WiFi initialisation). The result is stored in _device_name without a NULL check and then dereferenced in mdns_service_add and mdns_txt_item_t. Additionally, _device_name is a module-level pointer; if wendy_server_start is called twice (defensive scenario), the previous allocation is leaked.
Relevant snippet:
// components/wendy_server/src/wendy_server.c
void wendy_server_start(const char *device_name)
{
_device_name = device_name ? strdup(device_name) : NULL;
// ↑ strdup return value not checked
xTaskCreate(_server_task, ...);
}Remediation:
void wendy_server_start(const char *device_name)
{
free(_device_name); // handle re-entrant calls
_device_name = NULL;
if (device_name) {
_device_name = strdup(device_name);
if (!_device_name) {
ESP_LOGE(TAG, "strdup failed for device_name");
return; // or use a static fallback
}
}
xTaskCreate(_server_task, ...);
}Consider using a static char _device_name[CONFIG_WENDY_DEVICE_NAME_BUF_SIZE] instead of heap allocation to avoid the entire class of malloc/free issues in this constrained environment.
Controls violated: ISO 27001 A.8.28 (error handling), NIST SI-16 (memory protection), SOC2-A1 (availability — crash risk).
FINDING 5 — MEDIUM | ISO27001-A.8, NIST-SI-10
Device name accepted into s_device_name without validation before hostname construction
Description:
wendy_wifi_init copies the device name into s_device_name (module-level, 64 bytes) via strlcpy. The start_mdns_service function then processes s_device_name character by character and silently drops characters that are not [a-zA-Z0-9-_.]. This approach is defensive for the hostname construction, but the original unvalidated name is passed to wendy_server_start (Finding 2) and could produce an empty hostname if the entire name consists of dropped characters (e.g. a name of all Unicode/emoji). An empty mDNS hostname could result in unexpected mDNS behaviour or service registration failure without a clear error.
Relevant snippet:
// components/wendy_wifi/src/wendy_wifi.c
strlcpy(s_device_name, device_name ? device_name : CONFIG_WENDY_DEVICE_NAME_DEFAULT_PREFIX "-0000", sizeof(s_device_name));
// ... later in start_mdns_service, characters are filtered but result could be empty
hostname[out] = '\0';
ESP_LOGI(TAG, "mDNS hostname: '%s'", hostname);Remediation:
After the hostname construction loop, check out == strlen(prefix) (i.e. nothing was appended beyond the prefix). If so, append a MAC-derived suffix as fallback. More broadly, centralise validation at ingestion (Finding 1) so downstream functions can assume a clean input.
Controls violated: ISO 27001 A.8.28, NIST SI-10.
FINDING 6 — LOW | SOC2-CC7, ISO27001-A.12
Device name (potentially PII) logged unconditionally at INFO level
Description:
The device name provisioned by the user is logged at ESP_LOGI (INFO) level. Depending on deployment, serial/UART logs may be captured and stored. If the device name was chosen by a user to be personally identifying (e.g. their own name, "alice-livingroom"), this constitutes logging of PII. Under GDPR Article 5(1)(c) (data minimisation) and Article 5(1)(e) (storage limitation), logging PII without necessity or retention controls is a risk.
Relevant snippet:
// main/wendy_main.c
if (device_name[0]) {
ESP_LOGI(TAG, "device name: %s", device_name);
}
// and later:
ESP_LOGI(TAG, "device name: %s", device_name); // unconditional after BLE syncRemediation:
Use ESP_LOGD (DEBUG) instead of ESP_LOGI for the device name. In production builds, DEBUG logs are typically compiled out (CONFIG_LOG_DEFAULT_LEVEL). If INFO logging of the name is needed for diagnostics, consider logging only a hash or the suffix (last 4 chars) rather than the full name.
Controls violated: SOC2-CC7 (monitoring data minimisation), ISO 27001 A.8.11 (data masking), GDPR Article 5(1)(c).
FINDING 7 — LOW | ISO27001-A.8
Integer underflow possible in wendy_conf_copy_span when dest_size == 0
Description:
The expression dest_size - 1 is evaluated with size_t (unsigned) arithmetic. If dest_size == 0, this underflows to SIZE_MAX, making copy_size equal to src.size (since src.size < SIZE_MAX is always true). The subsequent memcpy would overflow dest, and dest[copy_size] = '\0' would write far out of bounds. The early-return guard only checks src.size == 0 || !src.data, not dest_size == 0.
Relevant snippet:
// components/wendy_conf/src/wendy_conf.c
void wendy_conf_copy_span(char *dest, size_t dest_size, struct wendy_conf_span src)
{
if (src.size == 0 || !src.data) {
if (dest_size > 0)
dest[0] = '\0';
return;
}
size_t copy_size = src.size < dest_size - 1 ? src.size : dest_size - 1;
// ↑ dest_size - 1 underflows if dest_size == 0Remediation:
if (dest_size == 0 || !dest) return;
if (src.size == 0 || !src.data) {
dest[0] = '\0';
return;
}
size_t copy_size = (src.size < dest_size - 1) ? src.size : dest_size - 1;Controls violated: ISO 27001 A.8.28 (secure coding, integer arithmetic safety).
FINDING 8 — INFORMATIONAL | ISO27001-A.8
MAC-derived suffix uses only 2 bytes (16 bits); low entropy identifier
Description:
The auto-generated device name suffix uses only the last 2 bytes of the BT MAC (addr[1], addr[0]), giving 65,536 possible names per prefix. In a deployment with many devices on the same LAN, this increases the probability of mDNS name collisions. This is not a direct security vulnerability but can lead to service advertisement conflicts and unexpected device misbehaviour.
Relevant snippet:
snprintf(s_device_name, sizeof(s_device_name),
"%s-%02x%02x", CONFIG_WENDY_DEVICE_NAME_DEFAULT_PREFIX,
addr[1], addr[0]);Remediation:
Consider using 3 or 4 bytes of the MAC for the suffix (e.g. %02x%02x%02x) to reduce collision probability, or implement mDNS conflict detection per RFC 6762 §9.
Controls violated: ISO 27001 A.8.28 (informational).
Compliance Summary
| Framework | Checked | Violations Found |
|---|---|---|
| SOC 2 (CC6, CC7, CC8, CC9, A1, C1, P-series) | ✅ | LOW — CC7 (unsanitised data in logs, potential PII logging) |
| ISO/IEC 27001:2022 (A.8, A.9, A.12) | ✅ | HIGH × 2, MEDIUM × 3, LOW × 2 — A.8.28 input validation and secure coding |
| PCI DSS v4.0 | ✅ | Not applicable — no payment/card data in scope |
| GDPR / Privacy | ✅ | LOW — device name (potential PII) logged at INFO level without necessity |
| HIPAA | ✅ | Not applicable — no health/medical data in scope |
| NIST SP 800-53 / CSF 2.0 (AC, AU, IA, SC, SI) | ✅ | MEDIUM × 2 — SI-10 (input validation), SI-16 (null-dereference/memory safety) |
Overall assessment: The PR is functionally sound and represents a reasonable design. The most important remediation before merge is adding a centralised device name validation function at ingestion (post-wendy_conf_copy_span) with an allowlist of [a-zA-Z0-9-] and a length cap, and guarding the wendy_ble_prov_get_device_name() return value for NULL before passing to strlcpy.
There was a problem hiding this comment.
Pull request overview
This PR wires the provisioned device name stored in the wendy_conf flash partition into Wendy Lite’s identity across BLE advertising, Wi-Fi/mDNS hostname, and service advertisement.
Changes:
- Add Kconfig options for device-name sizing and a default device-name prefix; remove the BLE-only device prefix.
- Plumb the device name through startup (
app_main→ BLE provisioning init → Wi-Fi init) and into mDNS + server TXT records. - Add a helper to safely copy
wendy_conf_spaninto a NUL-terminated buffer.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| main/wendy_main.c | Loads device name from wendy_conf, passes it into BLE init and Wi-Fi init, and waits for BLE stack readiness. |
| main/Kconfig.projbuild | Introduces shared device-name config (buffer size + default prefix) and removes BLE-specific prefix. |
| components/wendy_wifi/src/wendy_wifi.c | Stores device name for hostname/service startup, builds an mDNS hostname from it, and passes it to server startup. |
| components/wendy_wifi/include/wendy_wifi.h | Updates wendy_wifi_init signature to accept a device name. |
| components/wendy_wifi/CMakeLists.txt | Drops the wendy_ble_prov component dependency from wendy_wifi. |
| components/wendy_server/src/wendy_server.c | Accepts a device name for mDNS service instance naming and TXT records. |
| components/wendy_server/include/wendy_server.h | Updates wendy_server_start signature to accept a device name. |
| components/wendy_conf/src/wendy_conf.c | Adds wendy_conf_copy_span helper for copying spans into C strings. |
| components/wendy_conf/include/wendy_conf.h | Declares wendy_conf_copy_span. |
| components/wendy_ble_prov/src/wendy_ble_prov.c | Allows caller-supplied device name or MAC-derived generation; adds “BLE up” callback signaling. |
| components/wendy_ble_prov/include/wendy_ble_prov.h | Updates BLE provisioning API to accept an optional device name and adds on_ble_up callback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void wendy_conf_copy_span(char *dest, size_t dest_size, struct wendy_conf_span src) | ||
| { | ||
| if (src.size == 0 || !src.data) { | ||
| if (dest_size > 0) | ||
| dest[0] = '\0'; | ||
| return; | ||
| } | ||
| size_t copy_size = src.size < dest_size - 1 ? src.size : dest_size - 1; | ||
| memcpy(dest, src.data, copy_size); | ||
| dest[copy_size] = '\0'; | ||
| } |
| err = wendy_ble_prov_init(device_name[0] ? device_name : NULL, &ble_prov_cbs); | ||
| if (err != ESP_OK) { | ||
| ESP_LOGW(TAG, "BLE provisioning init failed"); | ||
| } | ||
|
|
||
| // Wait for BLE stack to come up | ||
| xEventGroupWaitBits( | ||
| s_events, | ||
| EVT_BLE_UP, | ||
| pdTRUE, pdFALSE, portMAX_DELAY); | ||
|
|
||
| ESP_LOGI(TAG, "BLE stack is up"); | ||
|
|
||
| if (!device_name[0]) { | ||
| strlcpy(device_name, wendy_ble_prov_get_device_name(), sizeof(device_name)); | ||
| ESP_LOGI(TAG, "device name: %s", device_name); | ||
| } |
| int rc = ble_hs_id_copy_addr(BLE_ADDR_PUBLIC, addr, NULL); | ||
| if (rc != 0) { | ||
| /* Fallback: use a fixed suffix */ | ||
| snprintf(s_device_name, sizeof(s_device_name), | ||
| "%s-0000", CONFIG_WENDY_BLE_PROV_DEVICE_PREFIX); | ||
| "%s-0000", CONFIG_WENDY_DEVICE_NAME_DEFAULT_PREFIX); | ||
| return; | ||
| } |
| const char prefix[] = "wendylt-"; | ||
| assert(sizeof(prefix) < sizeof(hostname)); | ||
| memcpy(hostname + out, prefix, sizeof(prefix) - 1); | ||
| out += sizeof(prefix) - 1; | ||
| for (size_t in = 0; s_device_name[in] && out < sizeof(hostname) - 1; in++) { |
| esp_err_t wendy_wifi_init(const char *device_name) | ||
| { | ||
| strlcpy(s_device_name, device_name ? device_name : CONFIG_WENDY_DEVICE_NAME_DEFAULT_PREFIX "-0000", sizeof(s_device_name)); | ||
|
|
During
wendy os install, the user can enter a device name. This name is stored in the wendy_conf partition (in flash) as the certificates and Wi-Fi credentials.This PR makes Wendy Lite use that name for BLE, Wi-Fi and service advertisement (mDNS).