fix(csrf): Fix SCRF vulnerability in OTA examples and libraries #11530
Conversation
👋 Hello me-no-dev, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@JLLeitschuh PTAL |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a CSRF vulnerability by adding authentication and CSRF header checks to OTA update endpoints across various examples.
- Enforces authentication on update routes
- Introduces CSRF header collection and validation in multiple files
- Applies similar security improvements in both WebUpdate and OTA updater code, as well as in the HTTPUpdateServer
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libraries/WebServer/examples/WebUpdate/WebUpdate.ino | Added authentication and CSRF header validation for OTA updates |
| libraries/Update/examples/OTAWebUpdater/OTAWebUpdater.ino | Integrated CSRF checks and authentication in OTA update flow |
| libraries/HTTPUpdateServer/src/HTTPUpdateServer.h | Updated CSRF header collection and verification in the update server |
Comments suppressed due to low confidence (3)
libraries/WebServer/examples/WebUpdate/WebUpdate.ino:64
- Consider adding an inline comment clarifying the CSRF header check logic, explaining the rationale for comparing 'Origin' with 'http://' concatenated with the Host header to improve future maintainability.
String origin = server.header(String(csrfHeaders[0]));
libraries/Update/examples/OTAWebUpdater/OTAWebUpdater.ino:74
- Adding an inline comment to document the CSRF validation steps here would help clarify why the origin is compared to 'http://' + host for maintaining secure updates.
String origin = server.header(String(csrfHeaders[0]));
libraries/HTTPUpdateServer/src/HTTPUpdateServer.h:111
- Consider sending an explicit HTTP error response (with an appropriate status code) when the CSRF check fails, to clearly communicate the failure to the client.
String origin = _server->header(String(csrfHeaders[0]));
Test Results 76 files 76 suites 13m 8s ⏱️ Results for commit 5f74e65. ♻️ This comment has been updated with latest results. |
Closes: https://github.com/espressif/arduino-esp32/security/advisories/GHSA-9vfw-wx65-c872