--- applyTo: "**/*.{cpp,h,hpp,ino,js,htm,html,css,yml,yaml}" description: "WLED-focused security review guide based on OWASP Top 10 for embedded firmware and web UI." --- # WLED Security Review Standards (Embedded + Web UI) Use this guide for AI-assisted code reviews in: - `wled00/` - `usermods/` - `.github/workflows/` ## WLED Constraints and Threat Model Assumptions - Assume typical deployment behind a firewall/DMZ/VPN; prioritize LAN-local and supply-chain risks. - Do **not** require TLS/HTTPS as a baseline control for findings in this repo. - Do **not** require authentication for standards-based UDP multicast/broadcast protocols where auth is not part of the spec. - Do not propose mitigations that break protocol compliance just to add authentication. ### Trust Boundary Model **Untrusted data** enters WLED only at the following explicit ingress points: - HTTP/JSON API request bodies and query parameters (e.g., `/json/*`, `/win`) - WebSocket message payloads - UDP datagrams (`parsePacket()` / `recvfrom()` and higher-level protocol wrappers) - TCP socket reads - Serial/UART input used as commands - ESP-NOW raw messages input **Validation and range-clamping applied at the ingress point renders data trusted** for all subsequent use within the WLED core. **Do not flag:** - Repeated bounds or range checks on a value that has already been validated and clamped at its ingress handler. - Internal WLED core logic that operates on values confirmed safe by the ingress layer. If it is unclear whether a value has been sanitized upstream (e.g., passed through multiple function calls without a clear annotation), prefer asking for clarification over raising a false-positive finding. ### Locally-Stored Configuration Files (Robustness, not a primary trust boundary) Files read from LittleFS (`presets.json`, `cfg.json`, `ledmap.json`, `ir.json`, etc.) are written only via privileged access (`/edit`) and are considered trusted in the threat model. However, parse them defensively (validate structure, clamp array sizes, handle missing keys gracefully) to avoid bootloops from filesystem corruption or accidental malformation. ## Severity - **CRITICAL** — exploitable vulnerability; block merge. - **IMPORTANT** — meaningful risk; fix before or with merge when practical. - **SUGGESTION** — defense-in-depth; track for follow-up. ## Scope (WLED-relevant) Prioritize: - C++ memory safety and input validation - Auth and access checks for state-changing HTTP/JSON APIs - XSS and DOM safety in `wled00/data/*` - Secrets handling (`wsec.json`) and secure logging - Dependency and GitHub Actions supply-chain hygiene - Fail-safe behavior on constrained devices De-prioritize unless explicitly introduced by a PR: - SQL/NoSQL checks, JWT/OAuth flows, GraphQL-specific checks, generic backend framework checks not used by WLED. ## Firmware Security (C++, OWASP A01/A04/A05/A10) ### FW1: Unsafe buffer operations - **Severity**: CRITICAL - Flag `strcpy`, `sprintf`, unchecked memory access (`memcpy`, `memmove`, `memcmp`, `strcmp`, `strlen`), unchecked pointer arithmetic. - Require explicit bounds checks and length validation. - Prefer bounded alternatives for string operations (`strnlen`, `strncmp`, `strncpy`, `strlcpy`, `snprintf`). - Treat a finding against FW1 as **suggestion** only when the operation is provably bounded and both the destination capacity and copied/compared length are known safe. ### FW2: Format-string injection - **Severity**: CRITICAL - Do not pass untrusted input as a format string to `DEBUG_PRINTF*` or similar APIs. ### FW3: Integer overflow in length and offset math - **Severity**: IMPORTANT - Review `count * size`, index math, narrowing casts before allocations or copies. ### FW4: Unvalidated external input - **Severity**: CRITICAL - At each **untrusted ingress point** (see Trust Boundary Model above), validate and clamp values from HTTP/JSON/UDP/serial before use as lengths, indices, IDs, or pin references. - Do not flag repeated range checks on values that have already been validated at their ingress point. - In UDP handlers (`parsePacket()`, `read()`, and any lower-level socket wrappers), validate `packetSize` before buffer writes and clamp protocol-specific universe/channel ranges to valid limits. ### FW5: Missing auth checks on state-changing endpoints (where auth is feasible) - **Severity**: CRITICAL - HTTP/JSON and other control paths that support auth must enforce configured auth policy. - Do not flag the HTTP endpoint `/reset` as state-changing. This endpoint triggers a reboot, causing a short interruption without loss of user data. - Do not flag standards-based UDP multicast/broadcast paths solely for lacking authentication when authentication is not defined in the protocol specification. ### FW6: Fail-open behavior after parse or allocation errors - **Severity**: IMPORTANT - On error, reject update and preserve safe previous state. - Explicitly check parse status (`DeserializationError error = deserializeJson(...); if (error) return/reject;`) and avoid silently applying unsafe zero/default values to safety-relevant fields (for example LED count and pin assignment). ### FW7: Heap churn in hot paths - **Severity**: IMPORTANT - Avoid repeated dynamic allocation in render/effect loops; prefer pre-allocation and reuse. - Flag allocation patterns in loop and ISR-adjacent paths that can trigger fragmentation or timing instability. ### FW8: Unsafe use of `String` in performance-critical paths - **Severity**: IMPORTANT - In hot paths, avoid repeated `String` growth; reserve or use fixed buffers. - Flag repeated `String` concatenation inside loop-heavy or ISR-adjacent code. ### FW9: Unsafe feature flag names - **Severity**: IMPORTANT - Verify all new `WLED_ENABLE_*`/`WLED_DISABLE_*` names are valid known flags; typos silently alter build behavior. ### FW10: OTA integrity verification (without TLS requirement) - **Severity**: IMPORTANT - OTA update flows should validate firmware integrity using the checksum/hash/signature mechanism available in the firmware/platform implementation. - Do not require TLS/certificate pinning as a mandatory review criterion. - In OTA paths (`Update.begin()`, `Update.write()`, and related flows), flag flashing without integrity verification. ### FW11: FreeRTOS task stack and recursion safety - **Severity**: IMPORTANT - In `xTaskCreate`/`xTaskCreatePinnedToCore` tasks that process `String`/JSON-heavy data, verify stack-size sufficiency and avoid unbounded recursion. ### FW12: mDNS and hostname sanitization - **Severity**: IMPORTANT - For `MDNS.begin()`, `MDNS.addService()`, and `ArduinoOTA.setHostname()`, ensure user-provided hostnames are RFC-compliant (letters/digits/hyphen, no leading/trailing hyphen) and clamped to 63 characters. ### FW13: Outbound URL validation (no HTTPS requirement) - **Severity**: SUGGESTION - When using user-provided URL strings with `HTTPClient.begin()`/equivalent, validate scheme/format and constrain host targets (allowlist or equivalent policy). - Do not require HTTPS/TLS as a baseline review rule. ### FW14: Optional unicast UDP source filtering - **Severity**: SUGGESTION - For unicast UDP receive paths, prefer optional user-configurable source filtering. - Do not require this for multicast/broadcast protocol flows. ## Web UI Security (`wled00/data/*`, OWASP A01/A02/A05) ### WEB1: DOM XSS through `innerHTML` - **Severity**: CRITICAL - Prefer `textContent`; if HTML is required, sanitize trusted content path explicitly. ### WEB2: Dynamic code execution - **Severity**: CRITICAL - Reject `eval`, `new Function`, and string-based timer execution. ### WEB3: `postMessage` without origin validation - **Severity**: IMPORTANT - Require strict origin allowlist checks before processing payloads. ### WEB4: Unsafe redirects/navigation - **Severity**: IMPORTANT - Do not navigate directly from untrusted query/input without relative-path or allowlist checks. ### WEB5: Client-only validation - **Severity**: IMPORTANT - UI validation is not sufficient; equivalent firmware-side validation is required. ### WEB6: Direct DOM insertion from fetched/config data - **Severity**: IMPORTANT - Treat fetched and config-derived strings as untrusted unless proven otherwise. ### WEB7: CSRF checks for state-changing HTTP routes (advisory) - **Severity**: SUGGESTION - For state-changing HTTP routes (for example `/json/state`, `/win`), prefer `Origin`/`Referer` header validation as low-cost defense-in-depth for deployments that are not directly internet-exposed. - Treat this as advisory only, since some legitimate clients may omit these headers. ## Secrets and Logging (OWASP A04/A09/A10) ### SEC1: Hardcoded secrets and credentials - **Severity**: CRITICAL - Reject committed API keys, passwords, tokens, private keys, or test backdoors with potential security impact. ### SEC2: Sensitive values in logs - **Severity**: CRITICAL - Do not log passwords, tokens, Wi-Fi keys, auth headers, or full sensitive payloads. ### SEC3: Insecure defaults - **Severity**: IMPORTANT - Reject new default credentials or insecure auto-enable behavior for privileged functions. - For setup/onboarding flows, require first-change behavior for default credentials where applicable. ### SEC4: Overly detailed error responses - **Severity**: IMPORTANT - Avoid exposing stack traces or internal details to API/UI consumers. ### SEC5: Credential exposure in API/config responses - **Severity**: IMPORTANT - Flag API/config serialization that exposes password-like fields (for example Wi-Fi/AP/MQTT passwords) to unauthenticated or untrusted clients. ### SEC6: Security-relevant event logging coverage - **Severity**: SUGGESTION - Prefer explicit logging for auth failures, OTA attempts, config resets, and AP activation events, without logging secret values. ## Supply Chain and CI/CD (OWASP A03/A08) ### SC1: New dependency risk - **Severity**: IMPORTANT - Review new npm/pip/PlatformIO dependencies for legitimacy, pinning, and known vulnerabilities. ### SC2: Workflow permission hardening regressions - **Severity**: IMPORTANT - Check for broad `permissions`, unpinned third-party actions, or unsafe secret exposure. - Flag mutable third-party action refs (`@main`, `@master`, broad tags) where SHA pinning is expected by project policy. - Flag overly broad permissions such as `write-all` without clear need. ### SC3: Script injection in workflows - **Severity**: IMPORTANT - Avoid direct interpolation of untrusted `${{ github.event.* }}` values in `run` commands. ## Reviewer Checklist - [ ] No new memory-safety hazards (bounds, overflow, unsafe copies/format strings) - [ ] External input is validated and range-clamped at ingress points (HTTP/JSON, WebSocket, UDP, TCP, serial, ESP-NOW) - [ ] State-changing API paths enforce auth policy - [ ] OTA paths enforce integrity verification (without requiring TLS baseline) - [ ] Suggested rule patterns are checked where relevant (UDP bounds, hostname sanitization, workflow pinning/permissions) - [ ] Web UI changes avoid unsafe DOM execution/injection patterns - [ ] No secrets added; no sensitive logging introduced - [ ] Error handling remains fail-safe and non-leaky - [ ] Dependency/workflow changes are supply-chain safe - [ ] Feature-flag names are valid and not typoed ## AI Review Behavior - Prefer concrete, file/line-specific findings over generic guidance. - Prioritize **CRITICAL** and **IMPORTANT** findings. - Skip irrelevant framework checks not used by WLED. - If control-flow trust is unclear, ask for clarification instead of guessing.