Skip to content

Many littles fixes - time for v1

Choose a tag to compare

@marklynch marklynch released this 06 Apr 00:23
· 258 commits to main since this release
Immutable release. Only release title and notes can be modified.

Fixed

  • Fixed register_requester directly accessing global s_pool_state and s_pool_state_mutex — register_requester_start now accepts pool_state_t * and SemaphoreHandle_t parameters, matching the dependency-injection pattern used by the message decoder; main.c passes &s_pool_state and s_pool_state_mutex at startup
  • Fixed send_uart_command in mqtt_commands.c bypassing bus_send_message — now calls bus_send_bytes (extracted from bus_send_message) so MQTT commands get TX-wait, TX LED flash, and hex logging consistent with all other bus writes; removed direct uart_write_bytes call and driver/uart.h include from mqtt_commands.c
  • Fixed race condition in dns_server_stop — replaced unreliable 100ms vTaskDelay + conditional vTaskDelete with a binary semaphore; the task signals the semaphore on all exit paths before calling vTaskDelete(NULL), and dns_server_stop blocks on it (3s timeout) rather than guessing when the task has finished
  • Fixed /status handler holding the pool state mutex for the entire JSON build — now takes a snapshot immediately after acquiring the mutex and releases it before any cJSON allocation, eliminating contention with the message decoder under load
  • Fixed potential silent truncation of MQTT broker URI — increased broker_uri static buffer from 192 to 256 bytes in mqtt_poolclient.c; the previous margin was tight enough that a max-length broker hostname with port would silently truncate the URI passed to the MQTT client
  • Fixed magic number 8 used as array size for channels_to_publish in handle_channel_status — replaced with MAX_CHANNELS so the array size stays in sync if the constant is ever changed
  • Fixed volatile bool used for s_mqtt_connected and s_mqtt_started in mqtt_poolclient.c — replaced with atomic_bool (<stdatomic.h>) which provides correct memory-ordering guarantees on all architectures; volatile provides no such guarantees and would be unsafe on multi-core targets
  • Fixed led_flash_rx/led_flash_tx blocking the tcp_bridge task for 50 ms via vTaskDelay — moved all flash work (set colour → delay → restore) into a dedicated low-priority led_flash_task; callers now post a led_flash_type_t to a depth-4 queue and return immediately; if the queue is full under burst conditions the flash is silently dropped rather than blocking bus message processing
  • Fixed tcp_bridge_stop deleting s_log_mutex while the task could still be inside tcp_bridge_vprintf holding it — replaced vTaskDelete(handle) with a cooperative stop: s_stop_requested flag causes the task to exit the loop cleanly, close sockets, and give a binary semaphore before calling vTaskDelete(NULL); tcp_bridge_stop waits on the semaphore (3s timeout with forced delete fallback) before restoring vprintf and deleting the mutex

Security

  • Fixed provisioning request buffer too small for max-length SSID (32 bytes) + password (63 bytes) + JSON overhead — increased HTTP_PROVISION_BUFFER_SIZE from 200 to 512 bytes
  • Fixed channel, light zone, and valve MQTT payloads using snprintf with unescaped name fields — replaced with cJSON construction so names containing ", \, or control characters produce valid JSON
  • Fixed handle_unknown heap-allocating a log buffer per unknown bus message — replaced with a stack buffer sized to 3 * BUS_MESSAGE_MAX_SIZE + 1 (769 bytes), eliminating heap fragmentation risk, silent OOM discard, and the signed integer overflow in 3 * len
  • Fixed malloc(0) and NULL pointer passed to esp_wifi_scan_get_ap_records when a WiFi scan returns zero APs — now returns an empty JSON array early before the malloc call
  • Fixed dangling pointer in mqtt_client_init — config.username and config.password were stack-allocated fields pointed to directly by the MQTT client config; they are now copied into static buffers (s_username, s_password) before assignment, matching the existing pattern used for broker_uri, device_id, and lwt_topic
  • Fixed provisioning AP password being logged in plaintext at INFO level — removed password from both ESP_LOGI calls in wifi_provisioning.c, preventing it from appearing on the serial console or being forwarded to any connected TCP log client
  • Fixed out-of-bounds read in tcp_bridge_vprintf — vsnprintf returns the would-be length when the buffer is too small, and that uncapped value was passed directly to send, reading past the end of the 256-byte stack buffer; capped to buffer size before sending
  • Fixed XSS via unescaped dynamic content in HTML responses — added html_escape() helper and applied it to WiFi SSID and MQTT broker in the home page, and broker/username in the MQTT config form; also converted the MQTT config form's html_fields[1536] fixed stack buffer to a dynamically-sized heap allocation
  • Fixed silent truncation of home page system info, WiFi, and MQTT rows — replaced fixed-size stack buffers (sys_table[1024], wifi_row[96], mqtt_row[256]) with heap-allocated buffers sized via snprintf(NULL, 0, ...), matching the pattern used by get_page_header/get_page_nav; also explicitly null-terminates ap_info.ssid before use
  • Fixed OTA handler accepting zero, negative, or oversized Content-Length values — added validation that rejects requests outside the range 1–OTA_MAX_FIRMWARE_SIZE (0x1E0000, matching the partition table) before entering the receive loop
  • Fixed race condition in handle_mode_control_cmd, handle_favourite_label, and handle_favourite_enable — mqtt_publish_favourite was called with a raw pointer to shared pool state after the mutex was released; all three now capture a snapshot inside the mutex and pass &state_snapshot, consistent with every other publish call in the decoder
  • Fixed out-of-bounds array writes in light zone register handlers (handle_light_zone_state, _color, _active, _multicolor, _name) — zone index derived from bus reg_id was not bounds-checked before indexing lighting[MAX_LIGHT_ZONES], allowing a crafted or malformed bus packet to corrupt adjacent fields in pool_state_t; dispatch table reg_end values tightened to base + MAX_LIGHT_ZONES - 1 and an explicit bounds check added in each handler

Full Changelog: v0.10.0...v1.0.0