fix(firmware): reject malformed setWifi and keep the WiFi password out of the log - #115
Open
isaka1022 wants to merge 3 commits into
Open
fix(firmware): reject malformed setWifi and keep the WiFi password out of the log#115isaka1022 wants to merge 3 commits into
isaka1022 wants to merge 3 commits into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
WifiConfigServer::handle_set_wifi()reads the two fields of asetWifimessage intoconst char*:ArduinoJson returns
nullptrfor a missing key. Both values are then formatted into the log and passed toconnect_wifi(), which takesstd::stringparameters, so asetWifimessage that omits either field aborts the device. Nothing is sent back to the app, so it sits onwifiConnectinguntil it times out.The same log line also writes the WiFi password to the serial console in clear text.
Changes
setWifimessage with a missingssidorpassword, and answerwifiConnectFailedso the app can show its error dialog instead of waiting.sdkconfig.defaults.localto.gitignore.firmware/CMakeLists.txtloads that overlay automatically when it exists, which makes it the natural place for per-deployment settings such asCONFIG_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 characteristice2e5e5e3-1234-5678-1234-56789abcdef0with a small Python BLE client that speaks the protocol directly.Before:
The client received no reply.
After:
The client decoded
wifiConnectFailedand the device stayed up.A well-formed
setWifistill connects normally; that path is unchanged.