martino/MARTe-Integrated-Components
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Implemented better testing and fixed skipepd frames

Browse Source
This commit is contained in:
Martino Ferrari
2026-07-01 16:39:34 +02:00
parent 7a326c5d78
commit 0bea41f866
46 changed files with 4358 additions and 1739 deletions
Download Patch File Download Diff File Expand all files Collapse all files
BUG_FIX_PLAN.md
+221
View File
@@ -0,0 +1,221 @@
# Bug Fix Plan — Security & Correctness Remediation
**Date:** 2026-06-26
**Based on:** `BUG_REPORT.md`
**Scope:** `Source/` and `Client/`
This plan organizes the ~60 findings from the audit into prioritized, dependency-ordered phases. Each phase is independently shippable. Phases are ordered by risk reduction: Critical remote-exploitable issues first, then High crash/OOB issues, then Medium robustness/DoS, then Low hardening.
---
## Guiding principles
1. **Fix root causes, not symptoms.** The integer-overflow-in-bounds-check pattern appears in 6+ places — fix the pattern, not each instance ad hoc. Introduce a shared `boundsCheck(off, count, elemBytes, bufLen)` helper (C++) and a `validateCount(count, elemSize, bufLen)` helper (Go) and use them everywhere.
2. **Defense in depth.** Origin checks + auth + input validation — not just one layer.
3. **No regressions.** After each phase, run the existing test suites (`make -f Makefile.gcc test`, `python3 -m unittest tests_py`, `go test ./...` in each Go module) and the E2E suite (`./Test/E2E/chain/run_chain_e2e.sh --skip-build`).
4. **Minimal blast radius.** Each fix is surgical to the file/function listed in the bug report. No refactors beyond what the fix requires.
---
## Phase 1 — Critical remote-exploitable fixes (ship first)
**Goal:** Eliminate drive-by takeover and remote heap corruption. All fixes are small and localized.
| # | Bug | File(s) | Fix | Est. effort | Depends on |
|---|-----|---------|-----|-------------|------------|
| 1.1 | CR-1: 1-byte heap OOB write in WS frame NUL-term | `WSServer.cpp:251` | Change `kRecvBuf = WS_MAX_RECV_PAYLOAD + 14u` → `+ 14u + 1u` | 5 min | — |
| 1.2 | CR-2: XSS via unescaped `src.addr` | `Client/udpstreamer/static/app.js:3503`; `Client/debugger/static/app.js:3549` | Wrap `src.addr` with existing `escHtml()` in `_statsKV` calls (or inside `_statsKV` itself) | 10 min | — |
| 1.3 | CR-3: WebSocket CSRF (Origin check disabled) | `Common/Client/go/wshub/hub.go:128`; `Source/Applications/StreamHub/WSServer.cpp:186-239` | **Go:** Replace `CheckOrigin: func(r *http.Request) bool { return true }` with a same-origin check (compare `Origin` header host to `Host` header). Add a configurable allowlist env var for non-local deployments. **C++:** Parse `Origin` header in `UpgradeHTTP`; reject if present and host doesn't match the listen address. | 30 min | — |
| 1.4 | CR-4: Unauthenticated command injection to MARTe2 | `Client/debugger/martecontrol.go:217-263` | Add an allowlist of permitted MARTe2 commands (`DISCOVER`, `TREE`, `INFO`, `LS`, `VALUE`, `TRACE`, `UNTRACE`); reject `FORCE`, `UNFORCE`, `PAUSE`, `RESUME`, `STEP`, `BREAK`, `MSG` unless an explicit `--enable-dangerous-commands` flag is set. Log all forwarded commands. | 1 h | 1.3 |
| 1.5 | CR-5: No auth on DebugService TCP | `DebugService.cpp:276` | (a) Bind TCP server to localhost by default (add `BindAddress` config key, default `127.0.0.1`). (b) Add an optional `AuthToken` config key; if set, require the first line from a client to be `AUTH <token>` before accepting commands. | 2 h | — |
**Validation:** `bash -n` on shell scripts; `go build ./...` in each Go module; `make -f Makefile.gcc core apps`; manual test: open browser console on a cross-origin page and confirm WS to `localhost:8090` is rejected; confirm a crafted 65536-byte WS frame no longer corrupts.
**Commit:** `fix(security): critical remote-exploitable fixes (CR-1..CR-5)`
---
## Phase 2 — High-severity crash / OOB / UAF fixes
**Goal:** Eliminate remote crash and memory-corruption vectors. These are the integer-overflow and concurrency bugs.
### 2A — Integer-overflow bounds checks (uniform pattern)
| # | Bug | File(s) | Fix |
|---|-----|---------|-----|
| 2A.1 | HI-1: DATA bounds check overflow | `UDPSourceSession.cpp:358`; `UDPStreamerClient.cpp:520` | Replace `off + elemsToRead * wireElemBytes > size` with 64-bit arithmetic. Add a `validateBounds(off, count, elemBytes, size)` static helper in `UDPSProtocol.h` and use it in both files. |
| 2A.2 | HI-2: Go unbounded allocations | `protocol.go:121, 229, 325` | Add `validateCount(count, elemSize, bufLen)` in `protocol.go`; call before every `make([]T, n)` that uses a network-derived count. Cap `NumElements()` at 1M. |
| 2A.3 | HI-3: `accumFill` overflow + size calc | `UDPStreamer.cpp:700, 738, 757, 857-860` | (a) Add `if (accumFill >= maxBatchCount) { flush; }` before the write at line 857. (b) Use `uint64` for `maxBatchCount * totalSrcBytes` size calculations. |
| 2A.4 | MD-4: `numRows * numCols` overflow | `UDPSourceSession.cpp:240, 346`; `protocol.go:121` | Use `static_cast<uint64>(numRows) * static_cast<uint64>(numCols)`; cap at 1M. |
| 2A.5 | MD-15: `pairCount * 16u` overflow | `Client/streamhub/Protocol.cpp:77, 117` | Check `pairCount > (len - off) / 16` before multiplication; use `ull` suffix. |
| 2A.6 | HI-6: `FD_SET` overflow | `UDPSServer.cpp:273, 308`; `UDPSClient.cpp:383` | Add `if (fd < FD_SETSIZE)` guard before each `FD_SET`; otherwise skip that client this cycle (or switch to `poll()`, which the codebase already uses elsewhere). |
**Est. effort:** 3 h (pattern is repetitive once the helper exists)
### 2B — Use-after-free and concurrency
| # | Bug | File(s) | Fix |
|---|-----|---------|-----|
| 2B.1 | HI-5: Broadcast vs FreeSlot UAF | `WSServer.cpp:345-366, 432-445` | `FreeSlot` must acquire `clients[idx].writeMutex` before setting `active=false` and deleting `sock`. This ensures `BroadcastText`/`BroadcastBinary` cannot dereference a freed socket. |
| 2B.2 | HI-9: TraceRingBuffer not thread-safe | `DebugCore.h:79-142` | Replace `volatile uint32 readIndex/writeIndex` with `Atomic<uint32>` (MARTe2 `Atomic::Load`/`Atomic::Store`). Ensure `Push` writes data before storing `writeIndex` (release ordering); `Pop` loads `writeIndex` before reading data (acquire ordering). |
| 2B.3 | HI-4: `ProcessSignal` unclamped memcpy + `forcedMask` OOB | `DebugServiceBase.cpp:310, 313-318` | (a) Clamp `size` to `sizeof(signalInfo->forcedValue)` (1024). (b) Cap the array-forcing loop at `min(nEl, 256)`. (c) Validate `nEl <= 256` in `RegisterSignal`. |
| 2B.4 | HI-7: Weak PRNG for WS handshake | `WSClient.cpp:29-31` | Replace `srand(time(nullptr))` + `rand()` with `std::random_device` or `getrandom()`/`/dev/urandom` read. |
| 2B.5 | HI-8: Global registry patching | `DebugServiceBase.cpp:217-242` | (a) Save original builders before patching (`item->GetObjectBuilder()`); store in a static array for restore on destruction. (b) Add a `PatchRegistry` config flag (default `true` for back-compat; document the implication). (c) Guard against double-patching (skip if already patched). |
**Est. effort:** 4 h
**Validation:** `make -f Makefile.gcc test` + `./Build/x86-linux/GTest/MainGTest.ex` + `./Build/x86-linux/Test/Integration/Integration/IntegrationTests.ex` + `python3 -m unittest tests_py` (in `Test/E2E/chain/`) + `go test ./...` (in each Go module). Craft a UDP packet with `numSamples=0x20000001` and confirm no crash. Run the E2E suite: `./Test/E2E/chain/run_chain_e2e.sh --skip-build`.
**Commit:** `fix(security): high-severity crash/OOB/UAF fixes (HI-1..HI-9)`
---
## Phase 3 — Medium-severity robustness / DoS / parser fixes
**Goal:** Harden input validation, fix reassembly logic, and improve WS RFC compliance.
### 3A — UDPS protocol hardening
| # | Bug | File(s) | Fix |
|---|-----|---------|-----|
| 3A.1 | MD-1: `recvMask` too small | `UDPSClient.cpp:544, 592-594` | Enlarge `recvMask` to 64 bytes (512 bits) to match the `totalFragments <= 512` cap. |
| 3A.2 | MD-2: No type matching in reassembly | `UDPSClient.cpp:548-555` | Add `type` field to `ReassemblySlot`; key on `counter && type`. |
| 3A.3 | MD-3: Signal name not null-terminated | `UDPSourceSession.cpp:219-223` | After `memcpy`, force `name[63]='\0'` and `unit[31]='\0'`. |
| 3A.4 | MD-6: No auth on UDP CONNECT | `UDPSServer.cpp:655-723` | Document trust boundary in `Docs/Protocol.md`. Optional: add a `ConnectToken` config key. |
| 3A.5 | MD-13: Reassembler unbounded map growth (Go) | `reassembler.go:41-89` | Add `maxSets = 1024` cap; reject new sets when full. |
| 3A.6 | LO-1: `totalFrags` overflow | `UDPSServer.cpp:541-542` | Validate `payloadSize <= maxPayloadSize * 65535` before the calculation. |
| 3A.7 | LO-17: `bufMutex.Create` unchecked | `UDPStreamer.cpp:119`; `UDPStreamerClient.cpp:149` | Check return value; `REPORT_ERROR` on failure. |
| 3A.8 | LO-19: Reassembler ticker panic | `reassembler.go:93` | Guard `if r.expiry <= 0 { r.expiry = 2 * time.Second }`. |
**Est. effort:** 2 h
### 3B — WebSocket and JSON robustness (C++ clients)
| # | Bug | File(s) | Fix |
|---|-----|---------|-----|
| 3B.1 | MD-16: `readU16`/`readU32` silent failure | `Client/streamhub/Protocol.cpp:21-38` | Change `readU16`/`readU32`/`readF64` to return `bool` (or set an `ok` flag); `ParseBinaryFrame` fails fast on any truncated read. |
| 3B.2 | MD-17: JSON injection in command builders | `Client/streamhub/Protocol.cpp:183-213` | Add a `jsonEscape(str)` helper; use it for all `%s` string interpolations. Switch to `std::string` to avoid truncation. |
| 3B.3 | MD-18: `strstr`-based JSON parsing | `Client/streamhub/Protocol.cpp:296-310, 495, 510` | Migrate `ParseSources`, `ParseZoom`, `ParseStats` to a real JSON parser. **ImGui:** add a minimal JSON parser or vendor a single-header library (e.g. nlohmann/json). **Qt:** use `QJsonDocument`. |
| 3B.4 | MD-19: WS RFC 6455 violations | `WSClient.cpp:204-223` | (a) Reject control frames with `payloadLen > 125`. (b) Implement `CONTINUATION` opcode reassembly (or at least log and drop with a clear message). (c) Echo `CLOSE` frame. |
| 3B.5 | MD-20: Handshake no timeout | `WSClient.cpp:290-301` | Set `SO_RCVTIMEO` to 5s on the socket before the handshake loop. |
| 3B.6 | MD-5: SHA1 latent overflow | `SHA1.h:50`; `WSFrame_client.h:113` | Add `if (len > 119u) return;` guard; use `uint64_t bitLen`; use `std::vector` instead of `new[]`/`delete[]`. |
| 3B.7 | MD-24: `parseCapture` panic | `Test/E2E/chain/client/main.go:140-171` | Add bounds checks before each read, mirroring `parsePush`. |
**Est. effort:** 4 h (3B.3 is the largest item — JSON parser migration)
### 3C — Go hub and debugger hardening
| # | Bug | File(s) | Fix |
|---|-----|---------|-----|
| 3C.1 | MD-10: No WS client cap | `hub.go:367-377` | Track `len(h.clients)`; reject above configurable max (default 32). |
| 3C.2 | MD-11: Silent data loss | `hub.go:346-358` | Add a `droppedCount` atomic counter per channel; expose via `Snapshot()`. |
| 3C.3 | MD-12: SSRF via `addSource` | `hub.go:83-96`; `sources.go:62-67` | Validate `addr` against a configurable allowlist (default: localhost + private RFC1918 ranges; reject link-local/metadata endpoints like `169.254.169.254`). |
| 3C.4 | MD-14: Index panic | `martecontrol.go:543` | Use `strings.TrimPrefix(line, "OK SERVICE_INFO ")` with a length check. |
| 3C.5 | LO-14: `stopCh` double-close | `martecontrol.go:182-189` | Use `sync.Once` for closing `stopCh`. |
| 3C.6 | LO-10: `unsafe.Pointer` aliasing | `hub.go:588-594` | Replace `float64ToBytes` with `binary.LittleEndian` put operations. |
| 3C.7 | LO-11: `+Inf` in JSON | `stats.go:115-116` | Guard `if avg > 0 { si.RateHz = 1.0 / avg } else { si.RateHz = 0 }`. |
**Est. effort:** 2 h
### 3D — TcpLogger and DebugService fixes
| # | Bug | File(s) | Fix |
|---|-----|---------|-----|
| 3D.1 | MD-7: `StringHelper::Copy` overflow | `TcpLogger.cpp:87` | Replace with `strncpy(entry.description, description, MAX_ERROR_MESSAGE_SIZE-1); entry.description[MAX_ERROR_MESSAGE_SIZE-1]='\0';` |
| 3D.2 | MD-8: `volatile` indices + lost wakeup | `TcpLogger.cpp:83-153, 157-158` | Use `Atomic::Load`/`Store` for `writeIdx`/`readIdx`; use `eventSem.ResetWait()` instead of `Wait`+`Reset`. |
| 3D.3 | MD-9: `printf` on RT thread | `TcpLogger.cpp:75-76` | Add a `MirrorToStdout` config key (default `false`); guard the `printf`/`fflush` behind it. |
| 3D.4 | MD-21: Stack buffer + shadowed member | `DebugService.cpp:438, 489` | Remove the local `udpsSampleBuf` (use the member); heap-allocate `cfgBuf`. |
| 3D.5 | MD-23: `configValidated` read without lock | `UDPStreamerClient.cpp:463` | Mark `volatile` or acquire `bufMutex` before reading. |
| 3D.6 | MD-22: Spinlock on RT path | `UDPStreamer.cpp:856, 947-976` | Minimize the RT-side critical section: swap a pointer instead of `memcpy` under the lock. Move the `memcpy` outside the lock (double-buffer pattern). |
| 3D.7 | LO-7: JSON escaping in DISCOVER | `DebugServiceBase.cpp:900-906` | Use the existing `EscapeJson` helper for signal names. |
| 3D.8 | LO-8: `EvaluateBreak` only element 0 | `DebugBrokerWrapper.h:61-86` | Document the limitation in the function comment. |
| 3D.9 | LO-9: `fprintf(stderr)` on init | `DebugBrokerWrapper.h:195-197` | Replace with `REPORT_ERROR`. |
**Est. effort:** 3 h
**Validation:** Full test suites + E2E. For 3B.3 (JSON parser migration), add unit tests for crafted JSON inputs (nested quotes, escaped chars, truncated payloads). For 3A.1/3A.2, add a unit test that sends duplicate high-index fragments and mixed-type same-counter fragments.
**Commit:** `fix(robustness): medium-severity input validation, parser, and DoS fixes (MD-1..MD-24)`
---
## Phase 4 — Low-severity hardening and documentation
**Goal:** Clean up latent bugs, fix doc mismatches, add missing hardening. These are non-urgent but improve code health.
| # | Bug | File(s) | Fix |
|---|-----|---------|-----|
| 4.1 | LO-2: `Stop()` TOCTOU | `WSServer.cpp:104-134` | Replace `Sleep(200ms)` with thread join. |
| 4.2 | LO-3: Spinlock priority inversion | `UDPSourceSession.h`; `WSServer.h` | Document that `FastPollingMutexSem` is only for very short critical sections on same-core RT configs. Consider `MutexSem` for non-RT-contended paths. |
| 4.3 | LO-4: `SignalBuffer` mod-0 | `SignalBuffer.h:36-41` | Guard `push`/`readLast`/`readRange` against `capacity == 0`. |
| 4.4 | LO-5: Misleading "Thread-safe" comment | `SignalBuffer.h:18` | Remove the claim or add internal locking. |
| 4.5 | LO-6: GAM type validation + doc | `SineArrayGAM.cpp:81`; `TimeArrayGAM.cpp:54`; `TimeArrayGAM.h:8,27` | Add `GetSignalType` checks; update `TimeArrayGAM.h` doc from `uint32` to `uint64`. |
| 4.6 | LO-12: Directory listing | `Client/webui/main.go:26` | Disable directory listings (return 404 for directories). |
| 4.7 | LO-13: No security headers | `Client/debugger/main.go:55`; `Client/udpstreamer/main.go`; `Client/webui/main.go` | Add a middleware that sets `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `Content-Security-Policy: default-src 'self'`. |
| 4.8 | LO-15: `host_`/`port_` race | `WSClient.cpp:48-58` | Protect with `sendMutex_` or make `atomic<uint16_t>` + `std::string` guarded by a small mutex. |
| 4.9 | LO-16: `ReadExactTCP` edge case | `UDPSClient.cpp:474-487` | Add a max-iterations guard. |
| 4.10 | LO-18: `RangeMin < RangeMax` validation | `UDPStreamer.cpp:403-404` | Validate when `quantType != None`; `REPORT_ERROR` if `rangeMax <= rangeMin`. |
**Est. effort:** 2 h
**Commit:** `fix(hardening): low-severity fixes, doc corrections, security headers (LO-1..LO-19)`
---
## Phase 5 — Cross-cutting refactors (optional, post-hardening)
These are not bug fixes but structural improvements that prevent the recurrence of the bug classes found in this audit.
| # | Refactor | Rationale | Est. effort |
|---|----------|-----------|-------------|
| 5.1 | Shared `validateBounds` / `validateCount` helpers | Centralizes the integer-overflow-prevention pattern; prevents future copy-paste bugs | 1 h |
| 5.2 | Real JSON parser in C++ clients (nlohmann/json or Qt's QJsonDocument) | Eliminates the entire class of `strstr`/`snprintf` JSON bugs (MD-16, MD-17, MD-18) | 4 h |
| 5.3 | `poll()`/`epoll` everywhere (replace all `select`+`FD_SET`) | Eliminates the `FD_SETSIZE` limitation entirely (HI-6) | 2 h |
| 5.4 | Auth framework for DebugService + web UIs | Token-based auth shared between the Go web UIs and the C++ DebugService; eliminates the "no auth anywhere" theme | 1 d |
| 5.5 | Fuzzing harness for UDPS protocol parsers | `libFuzzer` or `go-fuzz` harnesses that feed random bytes to `ParseConfig`/`ParseData`/`DecodeElems`/`ParseBinaryFrame`; catches future overflow variants | 1 d |
| 5.6 | Thread-sanitizer and address-sanitizer CI runs | `make CXXFLAGS="-fsanitize=address,undefined"`; `go test -race`; catches UAF and races automatically | 4 h |
---
## Verification checklist (run after each phase)
```bash
source env.sh
# C++ build + tests
make -f Makefile.gcc clean
make -f Makefile.gcc core apps test
./Build/x86-linux/GTest/MainGTest.ex
./Build/x86-linux/Test/Integration/Integration/IntegrationTests.ex
# Go tests (each module)
cd Common/Client/go && go vet ./... && go test ./... && cd -
cd Client/debugger && go vet ./... && go build ./... && cd -
cd Client/udpstreamer && go vet ./... && go build ./... && cd -
cd Test/E2E/chain/client && go vet ./... && go test ./... && cd -
# Python framework tests
cd Test/E2E/chain && python3 -m unittest tests_py && cd -
# Full E2E suite
./Test/E2E/chain/run_chain_e2e.sh --skip-build
# ASan/UBSan smoke test (after Phase 2+)
make -f Makefile.gcc clean
make -f Makefile.gcc CXXFLAGS="-fsanitize=address,undefined -g" core apps
./Build/x86-linux/GTest/MainGTest.ex
```
---
## Timeline summary
| Phase | Scope | Est. effort | Risk reduction |
|-------|-------|-------------|----------------|
| 1 | Critical remote-exploitable (CR-1..CR-5) | ~4 h | Eliminates drive-by takeover + heap corruption |
| 2 | High crash/OOB/UAF (HI-1..HI-9) | ~7 h | Eliminates remote crash + memory corruption |
| 3 | Medium robustness/DoS/parser (MD-1..MD-24) | ~11 h | Hardens input validation + RFC compliance |
| 4 | Low hardening/doc (LO-1..LO-19) | ~2 h | Code health + defense in depth |
| 5 | Cross-cutting refactors (optional) | ~3 d | Prevents recurrence of bug classes |
**Total (Phases 1-4):** ~24 h of focused work. Phase 5 is optional and can be scheduled separately.