Skip to content

fix(firmware): reject malformed setWifi and keep the WiFi password out of the log - #115

Open
isaka1022 wants to merge 3 commits into
m5stack:mainfrom
isaka1022:fix/ble-wifi-config-hardening
Open

fix(firmware): reject malformed setWifi and keep the WiFi password out of the log#115
isaka1022 wants to merge 3 commits into
m5stack:mainfrom
isaka1022:fix/ble-wifi-config-hardening

Conversation

@isaka1022

Copy link
Copy Markdown

Problem

WifiConfigServer::handle_set_wifi() reads the two fields of a setWifi message into const char*:

const char* ssid     = data["ssid"];
const char* password = data["password"];

mclog::tagInfo(_tag, "get wifi config: {} / {}", ssid, password);

ArduinoJson returns nullptr for a missing key. Both values are then formatted into the log and passed to connect_wifi(), which takes std::string parameters, so a setWifi message that omits either field aborts the device. Nothing is sent back to the app, so it sits on wifiConnecting until it times out.

The same log line also writes the WiFi password to the serial console in clear text.

Changes

  • Reject a setWifi message with a missing ssid or password, and answer wifiConnectFailed so the app can show its error dialog instead of waiting.
  • Log only the SSID.
  • Add sdkconfig.defaults.local to .gitignore. firmware/CMakeLists.txt loads that overlay automatically when it exists, which makes it the natural place for per-deployment settings such as CONFIG_STACKCHAN_SERVER_URL, but it is currently tracked and easy to commit by accident.

How this was tested

On a StackChan (CoreS3, ESP-IDF v5.5.4), sending {"cmd":"setWifi","data":{}} to the config characteristic e2e5e5e3-1234-5678-1234-56789abcdef0 with a small Python BLE client that speaks the protocol directly.

Before:

abort() was called at PC 0x421fcd63 on core 0
Backtrace: 0x4038ef11:0x3fcbfad0 0x4038eed9:0x3fcbfaf0 0x40396b66:0x3fcbfb10 ...
Rebooting...

The client received no reply.

After:

[WifiConfigServer] setWifi is missing ssid or password
[HAL-BLE] Config notify fragmented notify: total=67, mtu_payload=27, chunk_payload=17, packets=4
NimBLE: notify_tx event; conn_handle=1 attr_handle=22 status=0 is_indication=0   (x4)

The client decoded wifiConnectFailed and the device stayed up.

A well-formed setWifi still connects normally; that path is unchanged.

ArduinoJson returns nullptr for a missing key, and handle_set_wifi() passed
both values straight into std::string parameters, so a setWifi message
missing either field aborted the device instead of reporting a failure.
Reply wifiConnectFailed and keep running.
The password arrives over BLE and was written to the serial console in clear
text, where anyone with the USB port can read it.
CMakeLists.txt loads this overlay automatically when present, so it is the
natural place for deployment settings and credentials, but it was tracked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant