Drogheda/02-Architecture-Software/2026-09-18_Drogheda_htracker-Review_feature-test500unidades_v1.0.md

8.2 KiB
Raw Blame History

htracker — revisión de código, rama feature/test500unidades @ 2cdcb5e (18-sep-2026)

Archivos revisados: web_monitor.py (6 286 líneas), retranslator.py (1 466), device_metadata.py, web_monitor_ctl.sh, device_config.json. No revisados (no estaban en el set): auth.py, gps_logger.py — ver «Pendiente de verificar». Todos los hallazgos están verificados en el código (archivo:línea). Dueño del backend: Gibran.

Resumen ejecutivo (ES)

No es seguro seguir desplegando tal cual. Tres cosas primero, en este orden:

  1. XSS almacenado sin autenticación por UDP (C1) y entre clientes (C2). cell_id llega del $GPTXT sin validar y se inyecta con innerHTML en el panel de desconectadas, el mapa y los reportes; los nombres de unidad y de geocerca tampoco se escapan. Cualquiera en internet puede ejecutar JavaScript en el navegador del administrador con un datagrama a UDP 5010. Arreglo: re.fullmatch(r'H\d{4}', cell_id) en el retranslator + esc() en todos los sitios de inyección (una tarde).
  2. Secretos y datos en git (H1) — .session_secret, users.json, web_monitor.py.bak, telemetry_history.db: git rm --cached, .gitignore, rotar .session_secret (invalida sesiones), git filter-repo. Y limitar la edición de unidades/geocercas a las cell_id permitidas del usuario (H4).
  3. El reenvío a Wialon bloquea por paquete (H2) y reporta «ok» sin leer la respuesta (H3). Con 500 unidades a 1 Hz se descartan posiciones de forma sostenida mientras el tablero dice sano. Rehacer el forwarder (no esperar ACK síncrono, validar #ASD#1) antes del despliegue de 500.

Para las 15 unidades v5 de esta semana: C1 es el riesgo real hoy; H2/H3 no muerden a 30 unidades.


Detailed findings (EN, ranked)

CRITICAL

C1. Unauthenticated stored XSS via UDP: cell_id is never validated and is rendered unescaped. retranslator.py:691-695 returns parts[2] of any $GPTXT datagram verbatim; it becomes a key in every state dict and is emitted in /tmp/htracker_status.json. web_monitor.py:3780 (disconnected panel), 5555, 5607 (map InfoWindow) interpolate it into innerHTML without esc(). After DEVICE_TTL_S (300 s) the unit lands in disconnected and every open dashboard renders it; persists in htracker_device_state.json across restarts. Fix: validate (H\d{4}), drop and count otherwise; esc() every interpolated field.

C2. Authenticated cross-tenant stored XSS through unit names and geofence names. web_monitor.py:785-809 lets any logged-in user set unit_name for any cell_id (only .strip()); 952-960 same for geofence name. Rendered unescaped at 4963, 5069, 5155, 5141, 6115, 6128, 5556, 5608, 5653. Fix: escape at every sink; restrict unit_name to [\w \-\.]{1,40}; gate POST /api/units/{cell_id} on _user_allowed_cell_ids.

HIGH

H1. Secrets/data committed (.session_secret, users.json, web_monitor.py.bak, telemetry_history.db). Session cookies forgeable, hashes crackable offline. web_monitor_ctl.sh:38,68,69,98 hard-codes the public IP. H2. Wialon forwarder head-of-line blocks per worker. retranslator.py:511-516: sendall then blocking recv (15 s) per packet. Throughput per worker = 1/RTT; 64 workers ≈ 320–640 pkt/s; 500 units at 1 Hz = 500 pkt/s → sustained oldest-drop (386-402). One stalled socket freezes ~7 other units on that worker. Fix: write then drain ACKs non-blocking (select across the worker's sockets); add queue_depth stat. H3. "acked" counted without checking the response. 515-516: b'', #ASD#0, #ASD#-1 all count as ack → wialon_state='ok' while Wialon rejects. Fix: parse; only #ASD#1/#AD#1 = ok; b'' = disconnect. H4. Cross-tenant writes by design (769-824, "decisión de producto 2026-08-17") — with C2 this is privilege escalation. Gate on allowed cell_ids; vehicle-type icons full-access only. H5. SVG uploads served same-origin from /static unauthenticated (372, 611) = stored XSS. Drop SVG or serve with Content-Disposition: attachment + CSP sandbox.

MEDIUM

M1. Units with no fix are invisible: _parse_gga returns None on quality 0 (647), _RMC_RE needs A (217); NoFix chip unreachable. Mark seen + fix_mode='NoFix', skip only forward/odometer. M2. Hard speed gate ≥ 57.6 km/h (1169-1173) drops from Wialon and the local DB, not toggleable, not counted — the "healthy unit locked out" case of invariant #5. Make it GW_MAX_SPEED_KMH, count speed_rejects, keep _log_to_sql. M3. Unbounded state growth: _cleanup_thread (1033-1071) never prunes device_first_connected, device_total_online_s, device_last_seen_ever, device_km_*, device_disconnected; written every 5 s, reloaded on boot. Prune by last_seen_ever age (+ C1 validation). M4. km_24h resets at 00:00 UTC (570, 612, 1197 use utcnow()); everything else uses _TZ_CDMX. M5. KM reset offset (web_monitor.py:2596-2599, 140-142) corrupts km_24h on following days; store reset_at and apply only same local day. M6. Availability/disconnection reports bucket by UTC day (1090-1093); trips/CO2/track by local day → tabs disagree, "Hoy" misses 6 h. M7. SSE parses the whole status file per client every 2 s on the event loop (2629-2640); ~450 KB at 500 units × 20 dashboards = 10 parses/s stalling all handlers. One cached background reader; slim payload. M8. web_monitor_ctl.sh restart hangs while any dashboard is open (/events never completes; single SIGTERM at 79; port-in-use at 43-48). Add --timeout-graceful-shutdown 5, poll then kill -9. M9. Web monitor not under systemd, binds 0.0.0.0, cookie without secure (2755-2758). systemd unit, --host 127.0.0.1, secure=True behind TLS. M10. No logging in web_monitor; except … pass/return [] at 298-301, 313-314, 1106-1107, 1539-1540, 338-339, 2508-2509 → a DB permission/WAL problem shows as "0 % / no data" with HTTP 200. M11. CGNAT port reuse mis-attributes positions: addr→cell_id kept 300 s (1026-1029, 1353-1360). Expire at ~2× the $GPTXT period; refuse GGA if the addr's last $GPTXT is older.

LOW

L1 reset link ?email= unencoded (2800). L2 /logout is GET (2766). L3 Content-Disposition filename from unvalidated ids (2264, 2304, 2354). L4 Wialon packet timestamp = receipt time, not fix time (332). L5 CWD-relative paths (133, 149; web_monitor.py:57, 78) vs __file__-anchored ones; PrivateTmp=yes would hide /tmp/htracker_status.json. L6 one missed 5-min sample = "gap" (1371, 1405). L7 telemetry_history.db ≈ 30 MB/day at 500 units, no retention. L8 per-device INFO/ERROR lines (1335, 449, 499, 519) flood the collector at scale.

Verified OK

All SQL parametrized. Upload paths safe. Server-side f-string HTML escapes. CSRF mitigated by samesite=lax. Status JSON write atomic (tmp + rename). Per-cell_id read authorization enforced on every read endpoint. Anti-jump filter re-anchors after 3 rejects and forgets anchors on TTL — no permanent lockout from that filter. INSERT OR IGNORE + UNIQUE(cell_id, captured_at) dedupes.

Must verify in auth.py / gps_logger.py

HMAC session token compared with hmac.compare_digest; bcrypt/argon2/scrypt (not sha256); constant-time on unknown emails; reset tokens secrets.token_urlsafe(32), single-use, ≤ 1 h, stored hashed; lockout persistence. Index on gps_positions(device_cell_id, received_at) — without it fleet CO2/trips views do 60 full scans per request.

Improvements (not bugs)

  1. Cached SSE reader + delta payloads. 2. /run/htracker/ for the status file, passed explicitly to both processes. 3. web_monitor under systemd (Requires=htracker.service), 127.0.0.1, --proxy-headers, graceful timeout. 4. /healthz (status-file age, DB readable, forwarder drop rate). 5. Retention for telemetry_history.db, pruning of htracker_device_state.json. 6. 429/503 + Retry-After on heavy report ranges. 7. Structured logs; ship only [SUMMARY] + WARN. 8. UDP replay test harness with a fake Wialon returning #ASD#0 / closing / stalling.

Patch already prepared for the 0.5.1 firmware fields and $PHEV events: 02-Architecture-Software/htracker-patches/.