How plaintext credential storage happens in C embedded firmware (wifi.cpp) and how to fix it

How plaintext credential storage happens in C embedded firmware (wifi.cpp) and how to fix it

Introduction

The main/wifi.cpp file in this embedded firmware project handles a common real-world scenario: migrating legacy WiFi credentials stored in plaintext SD card files into the more secure ESP32 NVS (Non-Volatile Storage) subsystem. The intent was good — get credentials off the SD card and into encrypted NVS. But a subtle logic flaw in migrate_credentials_from_sd_if_present() meant that if the NVS write ever failed, the plaintext SD card files were silently left behind, fully readable by anyone with physical access to the card.

This is a critical issue for embedded and IoT devices, where physical access to storage media is often the primary attack surface. Unlike web applications where credentials transit a network, here the threat is someone popping open a device enclosure, pulling the SD card, and mounting it on a laptop.

The Vulnerability Explained

What the code was supposed to do

The function migrate_credentials_from_sd_if_present() reads legacy WiFi credentials (SSID and password) from plaintext files stored under /sdcard/apps/settings/config/ and writes them to NVS using credential_store_set(). After a successful NVS write, it calls config_delete() to remove the plaintext files from the SD card.

Here is the vulnerable logic (before the fix):

// VULNERABLE: deletion only happens inside the success branch
if (credential_store_set(legacy_ssid, legacy_password)) {
    bool deleted = false;
    if (config_bind_app("settings")) {
        bool del_ssid = config_delete(WIFI_SETTINGS_SSID_KEY);
        bool del_pass = config_delete(WIFI_SETTINGS_PASSWORD_KEY);
        config_unbind_app();
        deleted = del_ssid && del_pass;
    }
    if (deleted) {
        ESP_LOGI(TAG, "Migration complete; plaintext credentials removed from SD card");
    } else {
        // NVS now has the creds, but the SD plaintext files are still there.
        ESP_LOGW(TAG, "Migration copied to NVS, but failed to delete plaintext SD files");
    }
}
// If credential_store_set() returns false, we fall through here
// and do NOTHING to the SD card files. They stay. Forever.

The critical problem is on the very first line: the entire deletion block is wrapped inside if (credential_store_set(...)). If credential_store_set() returns false — for any reason (NVS partition full, hardware fault, flash wear, corruption) — the code silently exits the function without touching the SD card files. The plaintext SSID and password remain on disk, indefinitely.

Why FAT makes this worse

The code uses config_delete() which, on a FAT filesystem, performs a standard file deletion. FAT deletion does not overwrite file contents — it only marks the directory entry as free. The actual bytes of legacy_ssid and legacy_password remain in the flash sectors until those sectors happen to be reused. This means even a "successful" deletion may leave credentials forensically recoverable using tools like testdisk or direct sector-level reads.

Attack scenario

An attacker with physical access to the device:

1. Removes the SD card from the embedded device.
2. Mounts it on a standard Linux system: mount /dev/sdb1 /mnt/sdcard
3. If files still exist (migration failed): reads them directly from /mnt/sdcard/apps/settings/config/
4. If files were "deleted" but not overwritten: uses a sector-level reader or strings on the raw device image to recover the credential bytes.
5. Uses the recovered WiFi SSID and password to join the victim's network.

For an embedded device deployed in a home, office, or industrial environment, this gives the attacker WiFi access — a significant privilege escalation from physical proximity.

The Fix

The fix is elegant in its simplicity: decouple the SD card cleanup from the NVS migration result. The plaintext files should be deleted regardless of whether NVS storage succeeded.

Before vs. After

Before (vulnerable):

// Deletion is gated on NVS success
if (credential_store_set(legacy_ssid, legacy_password)) {
    if (config_bind_app("settings")) {
        bool del_ssid = config_delete(WIFI_SETTINGS_SSID_KEY);
        bool del_pass = config_delete(WIFI_SETTINGS_PASSWORD_KEY);
        config_unbind_app();
        // ...
    }
}
// If NVS fails → SD files untouched → credentials exposed

After (fixed):

// NVS migration is attempted, result is captured
bool nvs_ok = credential_store_set(legacy_ssid, legacy_password);
if (!nvs_ok) {
    ESP_LOGE(TAG, "Migration to NVS failed; will still attempt to remove plaintext SD files");
}

// ALWAYS delete SD card plaintext files, regardless of NVS outcome
bool deleted = false;
if (config_bind_app("settings")) {
    bool del_ssid = config_delete(WIFI_SETTINGS_SSID_KEY);
    bool del_pass = config_delete(WIFI_SETTINGS_PASSWORD_KEY);
    config_unbind_app();
    deleted = del_ssid && del_pass;
}
if (nvs_ok && deleted) {
    ESP_LOGI(TAG, "Migration complete...");
}

Why this works

The key structural change is extracting credential_store_set() into a standalone bool nvs_ok variable, then moving the deletion block outside and after the NVS call. The deletion now runs unconditionally. Even in the worst case — NVS write fails, SD delete fails — the code at least attempts to remove the plaintext files and logs a specific error for each failure mode, giving operators visibility into what happened.

The if (nvs_ok && deleted) check at the end is used only for logging the happy path; it no longer gates the deletion itself.

What this fix does NOT address (and what you should also do)

The fix closes the immediate logic flaw, but as noted above, FAT deletion does not securely erase data. For a complete defense-in-depth solution, consider:

1. Overwrite before delete: Write zeros to the credential files before calling config_delete().
2. Enable NVS encryption: ESP-IDF supports NVS encryption with a hardware-backed key. Without it, NVS is also readable if an attacker can dump flash.
3. Never write to SD in the first place: New devices should write credentials directly to NVS, eliminating the need for migration entirely.

Prevention & Best Practices

1. Never gate cleanup on success

This is the core lesson. When migrating sensitive data from an insecure location to a secure one, the cleanup of the insecure copy must be unconditional. Use this mental model:

"What is the worst state this code can leave the system in if any individual operation fails?"

In the original code, the worst state was "NVS fails silently, plaintext credentials persist forever." That's unacceptable.

2. Use hardware-backed secure storage

On ESP32, use NVS with flash encryption enabled. On Linux-based embedded systems, consider using the Linux kernel keyring, TPM-backed storage, or at minimum a file encrypted with a device-unique key. Plaintext files on removable media should never be the long-term home for credentials.

3. Secure erase on FAT

FAT does not zero data on deletion. If you must store sensitive data on FAT (SD cards, USB drives), implement a secure-erase wrapper:

// Pseudocode: overwrite before delete
void secure_delete_credential_file(const char* path) {
    FILE* f = fopen(path, "r+b");
    if (f) {
        fseek(f, 0, SEEK_END);
        long size = ftell(f);
        rewind(f);
        uint8_t zeros[64] = {0};
        for (long written = 0; written < size; written += sizeof(zeros)) {
            fwrite(zeros, 1, sizeof(zeros), f);
        }
        fflush(f);
        fclose(f);
    }
    remove(path);
}

4. Log all failure modes explicitly

The fixed code adds ESP_LOGE(TAG, "Migration to NVS failed; will still attempt to remove plaintext SD files"). This is good practice — operators monitoring device logs can detect and respond to migration failures before they become security incidents.

5. Relevant standards

• CWE-312: Cleartext Storage of Sensitive Information
• CWE-313: Cleartext Storage in a File or on Disk
• CWE-459: Incomplete Cleanup
• OWASP Mobile Top 10 – M9: Insecure Data Storage
• NIST SP 800-111: Guide to Storage Encryption Technologies

Key Takeaways

• The deletion of plaintext SD card credentials in migrate_credentials_from_sd_if_present() was gated on credential_store_set() success — meaning any NVS failure silently left credentials exposed.
• FAT filesystem deletion is not secure erasure — even after config_delete() runs, bytes remain on the SD card sectors until overwritten.
• The fix required only restructuring the control flow, not adding new cryptography — a reminder that logic bugs can be as dangerous as missing encryption.
• Embedded firmware migration code deserves the same security scrutiny as network-facing code — physical access is a realistic threat model for deployed IoT devices.
• Always log both success and failure paths for credential operations — the added ESP_LOGE call gives operators visibility into migration failures that were previously silent.

How Orbis AppSec Detected This

• Source: Legacy WiFi credentials read from plaintext files at /sdcard/apps/settings/config/ via config_bind_app("settings") and associated read functions into legacy_ssid and legacy_password buffers.
• Sink: config_delete(WIFI_SETTINGS_SSID_KEY) and config_delete(WIFI_SETTINGS_PASSWORD_KEY) in main/wifi.cpp:41 — the calls that should remove plaintext credential files from the SD card.
• Missing control: The deletion calls were nested inside if (credential_store_set(...)), making them conditional on NVS migration success. No cleanup occurred on the failure path.
• CWE: CWE-312 — Cleartext Storage of Sensitive Information; CWE-459 — Incomplete Cleanup.
• Fix: Extracted credential_store_set() result into bool nvs_ok, moved the deletion block outside the conditional so it always executes, and updated logging to reflect both success and failure states.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

The vulnerability in main/wifi.cpp is a textbook example of how good intentions — migrating credentials to a more secure store — can introduce a security regression through incomplete error handling. The original developer clearly understood that plaintext SD card storage was undesirable; the migration code exists precisely to address that. But by gating cleanup on success, the code created a silent failure mode where the very problem it was solving could persist indefinitely.

The fix is minimal in code size but significant in security posture: unconditional cleanup of the insecure copy, regardless of whether the secure copy succeeded. This principle — "always clean up the insecure state, even if the secure transition fails" — applies far beyond this specific file. Any time you're migrating data from a less-secure to a more-secure location, make sure the cleanup path is not contingent on the migration succeeding.

For embedded and IoT developers specifically: physical access is a real threat model. SD cards get removed. Devices get sold secondhand. Forensic recovery of FAT-deleted files is trivial. Treat credential storage on removable media with the same seriousness you'd give to a network-facing authentication endpoint.

References

• CWE-312: Cleartext Storage of Sensitive Information
• CWE-459: Incomplete Cleanup
• CWE-313: Cleartext Storage in a File or on Disk
• OWASP Mobile Security Testing Guide – Data Storage
• OWASP Cryptographic Storage Cheat Sheet
• ESP-IDF NVS Encryption Documentation
• Semgrep rules for credential storage
• fix: a migration function attempts to move historica... in wifi.cpp