Removed redundant text regarding destination capacity and length safety.
11 KiB
applyTo, description
| applyTo | description |
|---|---|
| **/*.{cpp,h,hpp,ino,js,htm,html,css,yml,yaml} | 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), validatepacketSizebefore 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
/resetas 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
Stringgrowth; reserve or use fixed buffers. - Flag repeated
Stringconcatenation 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/xTaskCreatePinnedToCoretasks that processString/JSON-heavy data, verify stack-size sufficiency and avoid unbounded recursion.
FW12: mDNS and hostname sanitization
- Severity: IMPORTANT
- For
MDNS.begin(),MDNS.addService(), andArduinoOTA.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), preferOrigin/Refererheader 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-allwithout clear need.
SC3: Script injection in workflows
- Severity: IMPORTANT
- Avoid direct interpolation of untrusted
${{ github.event.* }}values inruncommands.
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.