more protection for orange wifi firmware patch

[jasonlyc]

No description provided.

[j-sallyjin]

PR Review Summary

✅ What looks good

• Adds SHA-256 sidecar verification before copying Orange Wi-Fi firmware files, which directly reduces the risk of loading corrupted patch binaries.
• Builds a patch UID from sorted firmware hashes, so safe/reload state is tied to the actual patch content.
• Adds a guard to avoid repeatedly applying a firmware patch that appears to have failed to survive a reboot.
• Keeps the existing behavior of only reloading mt7996e when copied firmware differs from the current destination files.
• Linked issue coverage: firewalla/firecommit#8336 is mostly covered. The PR still applies the firmware patch before Wi-Fi config, persists it across reboot, skips reload when already updated, and keeps the existing NAPI CPU affinity reassignment path.

⚠️ Issues found

• Medium: platform/orange/OrangePlatform.js:210

  exec(\sha256sum ${srcPath}`)interpolates a filesystem path directly into a shell command. The current firmware filenames are repo-controlled, but this still creates avoidable shell parsing risk and will break on spaces or shell metacharacters. Since this is an integrity check, it should be especially boring and deterministic. Prefer hashing with Nodecrypto/fs`, or at least use a shell-escaped argument.

• Low: platform/orange/OrangePlatform.js:229

  The /dev/shm/firmware_patch_copy.<uid> flag is written before any copy or reload has actually completed. If copy or rmmod/modprobe fails during the same boot, _isPatchSafe() will continue allowing retries because the tmpfs copy flag exists, even though the safe flag was never written. That may be intentional for same-boot retry behavior, but the name and state model are a little misleading. Consider writing this only after copy succeeds, or naming it as an “attempt-in-current-boot” marker and documenting the intended retry semantics.

💡 Suggestions

• Consider validating the .sha256 file format before comparing, e.g. require exactly one 64-char lowercase hex digest. Right now extra content after trimming would simply cause a mismatch, which is safe, but a clearer log would help diagnose bad sidecar files.
• The safe flag currently means “firerouter was still alive 10 seconds after reload,” not necessarily “firmware loaded and Wi-Fi recovered.” If this protection is meant to prevent boot loops caused by bad firmware, that may be enough; if it is meant to prove functional Wi-Fi health, add a lightweight post-reload verification before writing firmware_patch_safe.<uid>.

Verdict

COMMENT

---

Repo: firewalla/firerouter
PR: #1925
Head SHA: a380a50a047ef166c21315df7fba4c544b0c2fd2
Checked at: 2026-06-17 19:47:46 CST