MARTe-Integrated-Components/BUG_REPORT.md

# Bug Report — Security & Correctness Audit

**Date:** 2026-06-26
**Scope:** `Source/` (C++ MARTe2 components + StreamHub app) and `Client/` (Go web clients, C++ ImGui/Qt desktop clients, JS web SPAs)
**Method:** Static source review of all network-facing, binary-parsing, concurrency, and web-serving code.

---

## Severity scale

| Level | Meaning |
|-------|---------|
| **Critical** | Remote code execution, drive-by takeover, or heap corruption reachable from the network. |
| **High** | Remote crash (OOM/panic/abort), out-of-bounds read/write, use-after-free, unauthenticated control of the real-time plant. |
| **Medium** | Data corruption, denial-of-service, parser confusion, RFC violations, races with bounded impact. |
| **Low** | Latent/fragile code, documentation mismatches, non-exploitable UB, missing hardening. |

---

## CRITICAL

### CR-1 — 1-byte heap OOB write in WebSocket frame NUL-termination

| Field | Value |
|-------|-------|
| File | `Source/Applications/StreamHub/WSServer.cpp:251, 315-316` |
| Severity | Critical |
| Type | Heap buffer overflow |
| Attack vector | Remote TCP (any client that completes the WS handshake) |

**Vulnerable code:**
```cpp
static const uint32 kRecvBuf = WS_MAX_RECV_PAYLOAD + 14u;  // 65536 + 14 = 65550
uint8 *buf = new uint8[kRecvBuf];        // valid indices 0..65549
...
// payload = frameStart + hdr.headerSize  (headerSize can be 14)
// plen   = up to WS_MAX_RECV_PAYLOAD (65536)
uint8 savedByte = payload[plen];         // payload[65536] == buf[65550] — OOB read
payload[plen] = '\0';                    // OOB write one byte past the heap allocation
```

**Root cause:** The comment `/* safe: buf has extra byte */` is incorrect. The buffer is exactly `65536 + 14 = 65550` bytes. A masked WebSocket frame with 64-bit extended length (`headerSize = 14`) and `payloadLen = 65536` fills the entire buffer; `payload[plen]` then writes one byte past the end.

**Impact:** Heap corruption — potential code execution depending on allocator/heap layout.

**Fix:** Allocate one extra byte:
```cpp
static const uint32 kRecvBuf = WS_MAX_RECV_PAYLOAD + 14u + 1u;
```

---

### CR-2 — XSS via unescaped `src.addr` in stats panel

| Field | Value |
|-------|-------|
| Files | `Client/udpstreamer/static/app.js:3503`; `Client/debugger/static/app.js:3549` |
| Severity | Critical |
| Type | Cross-site scripting (stored/reflected via server data) |
| Attack vector | Malicious or compromised StreamHub/DebugService feeding crafted source address |

**Vulnerable code:**
```js
body.innerHTML = `... ${_statsKV('Address', src.addr)} ...`;

function _statsKV(label, value, cls) {
  return `<div class="stats-kv"><span class="stats-k">${label}</span>` +
         `<span class="stats-v${cls ? ' ' + cls : ''}">${value}</span></div>`;
}
```

`_statsKV` interpolates `value` raw into HTML. `src.addr` originates from the WebSocket server's `sources`/`addSource` JSON. A crafted address such as `<img src=x onerror=alert(1)>` achieves arbitrary JavaScript execution in the browser, which can then drive the oscilloscope and trigger over the same WS connection.

**Impact:** Browser-side RCE → full control of the oscilloscope/trigger/MARTe2 debug interface via the compromised WS session.

**Fix:** Escape `src.addr` with the existing `escHtml()` helper before interpolation, or use `textContent`/`createElement`.

---

### CR-3 — WebSocket CSRF: `CheckOrigin` always returns `true` (Go) / no Origin check (C++)

| Field | Value |
|-------|-------|
| Files | `Common/Client/go/wshub/hub.go:128` (shared by `Client/udpstreamer` + `Client/debugger`); `Source/Applications/StreamHub/WSServer.cpp:186-239` |
| Severity | Critical |
| Type | Cross-Site WebSocket Hijacking (CSWSH / CSRF) |
| Attack vector | Drive-by web page |

**Vulnerable code (Go):**
```go
var upgrader = websocket.Upgrader{
    ReadBufferSize:  4096,
    WriteBufferSize: 64 * 1024,
    CheckOrigin:     func(r *http.Request) bool { return true },
}
```

**Vulnerable code (C++):** `UpgradeHTTP` parses `Sec-WebSocket-Key` and computes the accept hash but never reads or validates the `Origin` header.

**Impact:** Any malicious web page visited by the user can open a WebSocket to `ws://localhost:8080/ws` (or `:9090`, or the C++ hub's `:8090`) and send JSON commands — `addSource`, `arm`, `setTrigger`, `removeSource`, and in the debugger `PAUSE`/`RESUME`/`MSG`/`TRACE`/`FORCE`/`BREAK` against the live MARTe2 instance. Combined with CR-4 this is a full drive-by takeover of the plant controller.

**Fix:** Validate `Origin` against a configurable allowlist (same-host at minimum); reject cross-origin upgrade requests.

---

### CR-4 — Unauthenticated arbitrary command injection to MARTe2

| Field | Value |
|-------|-------|
| File | `Client/debugger/martecontrol.go:217-263` |
| Severity | Critical |
| Type | Command injection / unauthorized control |
| Attack vector | Drive-by web page (via CR-3) or direct TCP |

**Vulnerable code:**
```go
case "cmd":
    data, _ := env["data"].(map[string]interface{})
    if data == nil { return }
    cmd, _ := data["cmd"].(string)
    if cmd != "" {
        m.trackForcedCmd(cmd)
        m.SendCommand(cmd)   // raw string sent to MARTe2 TCP control
    }
```

**Impact:** Any web page (via CR-3) or any TCP client can send `FORCE`, `TRACE`, `PAUSE`, `RESUME`, `STEP`, `MSG`, `BREAK` to the real-time control system.

**Fix:** Require authentication (token-based) for command-sending WS messages; validate `cmd` against an allowlist of known MARTe2 commands; fix the Origin check (CR-3).

---

### CR-5 — No authentication on DebugService TCP command interface

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/DebugService/DebugService.cpp:276` |
| Severity | Critical |
| Type | Missing authentication / unauthorized control |
| Attack vector | Direct TCP to port 8080 |

**Vulnerable code:**
```cpp
BasicTCPSocket *newClient = tcpServer.WaitConnection(TimeoutType(100));
```

Any TCP client that connects can send `FORCE` (overwrite signal memory in the RT app), `PAUSE` (halt the RT loop), `STEP`, `TRACE`, and `MSG` (invoke any message handler on any ORD object — see HI-9). There is no authentication, TLS, or access control.

**Impact:** Remote control of the real-time plant: forced signal values, paused execution, arbitrary message dispatch.

**Fix:** Bind to localhost by default; add a shared-secret token or TLS gate; restrict `MSG` to whitelisted destinations.

---

## HIGH

### HI-1 — Integer overflow in DATA bounds check → heap OOB read

| Field | Value |
|-------|-------|
| Files | `Source/Applications/StreamHub/UDPSourceSession.cpp:358`; `Source/Components/DataSources/UDPStreamerClient/UDPStreamerClient.cpp:520` (same bug mirrored) |
| Severity | High |
| Type | Integer overflow → out-of-bounds read |
| Attack vector | Single crafted UDP DATA packet |

**Vulnerable code (StreamHub):**
```cpp
// numSamples read directly from attacker-controlled UDP payload (line 333-338)
if (pm == UDPS_PUBLISH_ACCUMULATE) {
    memcpy(&numSamples, payload + offset, 4u);
    if (numSamples == 0u) { numSamples = 1u; }
}
uint32 elemsToRead = (pm==ACCUMULATE && numElements==1) ? numSamples : numElements;
if (off + elemsToRead * wireElemBytes > size) { return; }  // 32-bit multiply wraps
```

`elemsToRead` is fully attacker-controlled via `numSamples`. `elemsToRead * wireElemBytes` is a `uint32` multiplication that can wrap to a small value (e.g. `0x20000001 * 8 = 0x8`), causing the bounds check to pass. The subsequent `DecodeElems` loop then reads far past the payload buffer.

**Impact:** Heap OOB read → crash or information leak.

**Fix:** Use 64-bit arithmetic for the bounds check:
```cpp
uint64 bytesNeeded = static_cast<uint64>(off)
    + static_cast<uint64>(elemsToRead) * static_cast<uint64>(wireElemBytes);
if (bytesNeeded > static_cast<uint64>(size)) { return; }
```

---

### HI-2 — Unbounded allocations from a single crafted UDP datagram (Go decoder)

| Field | Value |
|-------|-------|
| File | `Common/Client/go/udpsprotocol/protocol.go:121, 229, 325` |
| Severity | High |
| Type | Denial of service (OOM/panic) |
| Attack vector | Single crafted UDP CONFIG or DATA packet |

**Vulnerable code:**
```go
// protocol.go:121 — NumElements()
func (s SignalInfo) NumElements() int {
    r := int(s.NumRows); c := int(s.NumCols)
    if r == 0 { r = 1 }; if c == 0 { c = 1 }
    return r * c   // no overflow check; (2^32-1)^2 overflows int64 → negative → panic
}

// protocol.go:229 — ParseConfig()
numSigs := binary.LittleEndian.Uint32(payload[0:4])
sigs := make([]SignalInfo, 0, numSigs)   // numSigs=0xFFFFFFFF → ~480 GB alloc → OOM

// protocol.go:325 — ParseData()
numSamples := int(binary.LittleEndian.Uint32(payload[8:12]))
samples := make([]DataSample, numSamples)   // numSamples=0xFFFFFFFF → OOM
```

**Impact:** A single 17-byte UDP datagram crashes the Go web UI / chain client (panic or OOM kill).

**Fix:** Validate `numSigs`/`numSamples`/`NumRows*NumCols` against `len(payload)/elementSize` before allocating. Cap `NumElements()` at a sane maximum (e.g. 1M).

---

### HI-3 — `accumFill` increment before bounds check → buffer overflow

| Field | Value |
|-------|-------|
| File | `Source/Components/DataSources/UDPStreamer/UDPStreamer.cpp:857-860` |
| Severity | High |
| Type | Buffer overflow |
| Attack vector | Configuration-driven (signal/payload size mismatch) |

**Vulnerable code:**
```cpp
uint8 *slot = accumBuffer + (accumFill * totalSrcBytes);
(void) MemoryOperationsHelper::Copy(slot, memory, totalSrcBytes);
accumTimestamps[accumFill] = ts;
accumFill++;                                  // incremented unconditionally
// flush check at 866-871 only fires if payload-size/time conditions hit;
// if they don't and accumFill == maxBatchCount, the next call writes past the buffer
```

Related: the `maxBatchCount * totalSrcBytes` buffer-size calculation at line 700 (and 738, 757) also overflows `uint32`, allocating a too-small buffer.

**Impact:** Heap buffer overflow in the Accumulate publishing path.

**Fix:** Check `accumFill >= maxBatchCount` before writing; force-flush if so. Use `uint64` for the size calculation.

---

### HI-4 — `ProcessSignal` memcpy uses unclamped broker `size` → OOB read from `forcedValue[1024]`

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/DebugService/DebugServiceBase.cpp:310, 313-318` |
| Severity | High |
| Type | Out-of-bounds read |
| Attack vector | Forcing a signal whose runtime byte size exceeds the `ForceSignal`-validated size |

**Vulnerable code:**
```cpp
if (nEl <= 1u) {
    memcpy(signalInfo->memoryAddress, signalInfo->forcedValue, size);
    // size comes from broker GetCopyByteSize, not from ForceSignal validation (line 420)
```

`ForceSignal` rejects signals > 1024 bytes (line 420), but the runtime `size` comes from `GetCopyByteSize(j)` and can exceed 1024, reading past `forcedValue[1024]`. Additionally, the array-forcing loop at line 313-318 reads `forcedMask[e >> 3]` for `e` up to `nEl`; `forcedMask` is only 32 bytes (256 bits), so `nEl > 256` causes an OOB read of `forcedMask`. `ForceSignal` at line 445-448 sets bits up to `numberOfElements`, which can exceed 256, also writing past `forcedMask`.

**Impact:** OOB heap read; potential OOB write to `forcedMask`.

**Fix:** Clamp `size` to `sizeof(signalInfo->forcedValue)` in `ProcessSignal`; validate `nEl <= 256` in `RegisterSignal`/`ForceSignal`; cap the array-forcing loop at `min(nEl, 256)`.

---

### HI-5 — Race / use-after-free: BroadcastText vs FreeSlot

| Field | Value |
|-------|-------|
| File | `Source/Applications/StreamHub/WSServer.cpp:345-366` (Broadcast) vs `432-445` (FreeSlot) |
| Severity | High |
| Type | Use-after-free / data race |
| Attack vector | Triggered by client disconnect during broadcast |

**Vulnerable code (BroadcastText):**
```cpp
void WSServer::BroadcastText(const char *json, uint32 len) {
    for (uint32 i = 0u; i < WS_MAX_CLIENTS; i++) {
        if (!clients[i].active) { continue; }          // no lock held
        (void) clients[i].writeMutex.FastLock();
        if (clients[i].active) {                        // re-check under writeMutex
            (void) SendFrame(clients[i], ...);          // uses clients[i].sock
        }
        clients[i].writeMutex.FastUnLock();
    }
}
```

**Vulnerable code (FreeSlot):**
```cpp
void WSServer::FreeSlot(uint32 idx) {
    (void) clientsMutex.FastLock();           // holds clientsMutex, NOT writeMutex
    if (clients[idx].active) {
        clients[idx].active = false;
        if (clients[idx].sock) {
            clients[idx].sock->Close();
            delete clients[idx].sock;          // frees the socket
            clients[idx].sock = NULL;
        }
    }
    clientsMutex.FastUnLock();
}
```

`FreeSlot` (read loop, on disconnect) holds `clientsMutex` and `delete`s `sock`. `BroadcastText`/`BroadcastBinary` (push thread) holds only `writeMutex` and dereferences `sock`. There is no mutual exclusion on `active`/`sock` between these two code paths.

**Impact:** Use-after-free → crash or potential code execution.

**Fix:** `FreeSlot` must acquire `writeMutex` before modifying `active`/`sock`, or `BroadcastText`/`BroadcastBinary` must hold `clientsMutex` during the iteration.

---

### HI-6 — `FD_SET` with `fd >= FD_SETSIZE` → stack buffer overflow

| Field | Value |
|-------|-------|
| Files | `Source/Components/Interfaces/UDPStream/UDPSServer.cpp:273, 308`; `Source/Components/Interfaces/UDPStream/UDPSClient.cpp:383` |
| Severity | High |
| Type | Stack buffer overflow |
| Attack vector | Many open file descriptors (many TCP clients, high ulimit) |

**Vulnerable code:**
```cpp
int fd = tcpClients[i]->GetReadHandle();
fd_set rset;
FD_ZERO(&rset);
FD_SET(fd, &rset);               // if fd >= FD_SETSIZE (typically 1024), stack buffer overflow
```

`FD_SET` writes to a fixed-size stack bitmap (`fd_set` is typically 1024 bits / 128 bytes). If `fd >= FD_SETSIZE`, it writes past the bitmap. The multicast listener path already uses `poll()` but existing-client polling reverts to `select()`.

**Impact:** Stack buffer overflow when the process has many open FDs.

**Fix:** Check `fd < FD_SETSIZE` before calling `FD_SET`, or switch to `poll()`/`epoll` (which the codebase already uses elsewhere).

---

### HI-7 — Weak PRNG for WebSocket handshake key

| Field | Value |
|-------|-------|
| File | `Client/streamhub/WSClient.cpp:29-31` |
| Severity | High |
| Type | Weak randomness |
| Attack vector | Predict key → MITM handshake |

**Vulnerable code:**
```cpp
static std::string base64Key() {
    uint8_t raw[16];
    srand(static_cast<unsigned>(time(nullptr)));       // second-granularity, global RNG reseed
    for (int i = 0; i < 16; i++) {
        raw[i] = static_cast<uint8_t>(rand() & 0xFF);  // rand() often only 15-32 bits entropy
    }
    ...
}
```

`srand(time(nullptr))` is predictable to second granularity. `rand()` is not cryptographically secure. An attacker who knows the approximate connection time can predict the `Sec-WebSocket-Key`. Calling `srand` on every `base64Key()` invocation also reseeds the global C RNG, affecting any other `rand()` consumer.

**Impact:** Predictable WS handshake key; potential MITM. Global RNG pollution.

**Fix:** Use a CSPRNG (`getrandom()`, `/dev/urandom`, or `std::random_device`).

---

### HI-8 — Global registry patching of all brokers

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/DebugService/DebugServiceBase.cpp:217-242` |
| Severity | High |
| Type | Design-level global state mutation |
| Attack vector | N/A (correctness/design) |

**Vulnerable code:**
```cpp
void DebugServiceBase::PatchRegistry() {
    PatchItemInternal("MemoryMapInputBroker",      new DebugMemoryMapInputBrokerBuilder());
    PatchItemInternal("MemoryMapInputOutputBroker", new DebugMemoryMapInputOutputBrokerBuilder());
    // ... 10 more broker types
}

static void PatchItemInternal(const char8 *originalName, ObjectBuilder *debugBuilder) {
    ClassRegistryItem *item = ClassRegistryDatabase::Instance()->Find(originalName);
    if (item != NULL_PTR(ClassRegistryItem *)) {
        item->SetObjectBuilder(debugBuilder);   // globally replaces the builder
    }
}
```

This globally replaces the `ObjectBuilder` for all standard broker classes process-wide, silently, on `Initialise()`. Implications:
- The `new Debug*BrokerBuilder()` allocations are never freed (memory leak, intentional for process lifetime).
- Original builders are not saved/restored — if `DebugService` is destroyed, patched builders remain.
- A second `DebugService` instance double-patches; the first's builders are leaked.
- Any `dynamic_cast` to the original broker type breaks (gets a debug wrapper instead).

**Fix:** Document prominently; make patching opt-in rather than automatic on `Initialise()`; save/restore original builders.

---

### HI-9 — `TraceRingBuffer` not thread-safe

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/DebugService/DebugCore.h:79-142` |
| Severity | High |
| Type | Data race |
| Attack vector | N/A (correctness) |

**Vulnerable code:**
```cpp
uint32 Push(const uint8 *data, uint32 size) {   // called from RT broker thread under tracePushMutex
    ...
    uint32 w = writeIndex;                       // volatile uint32 — no memory barrier
    ...
    writeIndex = next;                           // volatile write
}

uint32 Pop(uint8 *dst, uint32 maxBytes) {        // called from Streamer thread, NO LOCK
    uint32 r = readIndex;                        // volatile read
    uint32 w = writeIndex;                       // volatile read — can be torn/stale
    ...
}
```

`Push` (RT broker thread, under `tracePushMutex`) and `Pop` (streamer thread, **no lock**) communicate via `volatile uint32` indices. `volatile` does not provide atomicity or memory ordering on most architectures (it only prevents compiler reordering, not CPU reordering). A torn/stale read of `writeIndex` can cause `Pop` to read partially-written data or miss entries.

**Impact:** Trace data corruption / lost trace samples / potential torn reads.

**Fix:** Use `Atomic::Load`/`Atomic::Store` for `readIndex`/`writeIndex`, or acquire `tracePushMutex` in `Pop` as well (adds RT path latency — prefer a proper lock-free SPSC ring).

---

## MEDIUM

### MD-1 — Fragment reassembly `recvMask` too small for `totalFragments` up to 512

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/UDPStream/UDPSClient.cpp:544, 592-594, 630-636` |
| Severity | Medium |
| Type | Logic bug / data corruption |

`totalFragments` is capped at 512 (line 544), but `recvMask` is only 32 bytes (256 bits). For `fragIdx >= 256`, the duplicate check at line 594 is skipped (`byteIdx >= 32`). An attacker can send the same high-index fragment repeatedly; each is accepted (overwriting the same data), `receivedFragments` is incremented each time, and `DeliverAssembled` is triggered prematurely with an incomplete reassembly — delivering partially-filled payload data to the application.

**Fix:** Cap `totalFragments` at 256, or enlarge `recvMask` to 64 bytes (512 bits).

---

### MD-2 — Fragment reassembly: no type matching → CONFIG/DATA type confusion

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/UDPStream/UDPSClient.cpp:548-555` |
| Severity | Medium |
| Type | Logic bug / data corruption |

Reassembly slots are keyed only on `counter`, not `type`. The protocol does not guarantee disjoint counter spaces for DATA and CONFIG. An attacker (or buggy server) can send DATA and CONFIG fragments with the same counter; they mix into the same reassembly buffer. The `type` field from the first-arriving fragment determines delivery destination, but the payload is a mixture.

**Fix:** Include `type` in the slot lookup: `reassemblySlots[i].counter == counter && reassemblySlots[i].type == hdr->type`.

---

### MD-3 — Signal name/unit not null-terminated after `memcpy` → intra-struct OOB read

| Field | Value |
|-------|-------|
| File | `Source/Applications/StreamHub/UDPSourceSession.cpp:219-223` |
| Severity | Medium |
| Type | Out-of-bounds read (intra-struct) |

After `memcpy(&sigDescs_[i], payload + ..., UDPS_SIGNAL_DESC_SIZE)`, `name[64]` and `unit[32]` are not force-null-terminated. If the CONFIG payload fills all 64 name bytes without `\0`, subsequent `strcmp(descs[s].name, ...)` (line 968) and `snprintf(key, ..., descs[i].name)` (line 916) read past `name` into `typeCode`, `quantType`, etc. until a zero byte is found.

**Fix:** After the memcpy:
```cpp
sigDescs_[i].name[UDPS_MAX_SIGNAL_NAME - 1u] = '\0';
sigDescs_[i].unit[UDPS_MAX_UNIT_LEN - 1u] = '\0';
```

---

### MD-4 — `numRows * numCols` integer overflow in CONFIG/DATA parsing

| Field | Value |
|-------|-------|
| Files | `Source/Applications/StreamHub/UDPSourceSession.cpp:240, 346`; `Common/Client/go/udpsprotocol/protocol.go:121` |
| Severity | Medium |
| Type | Integer overflow → data corruption |

`numRows` and `numCols` are `uint32` from the CONFIG payload. Their product can overflow `uint32` (e.g. `0x10000 * 0x10000 = 0`), producing a too-small scratch buffer and wrong `numElements` for DATA decoding.

**Fix:** Use 64-bit multiplication and cap against a sane maximum.

---

### MD-5 — SHA1 stack buffer overflow for inputs > 119 bytes (latent)

| Field | Value |
|-------|-------|
| Files | `Source/Applications/StreamHub/SHA1.h:50`; `Client/streamhub/WSFrame_client.h:113` |
| Severity | Medium |
| Type | Stack buffer overflow (latent) |

```cpp
uint8 msg[128];
memcpy(msg, data, len);   // if len > 120, overflows msg[128]
msg[len] = 0x80u;         // OOB if len >= 128
```

Currently only called with the WS key+GUID (~60 bytes), so not exploitable today. `WSFrame_client.h:113` also has `uint32_t bitLen = len * 8u` which truncates for `len > 512MB`, and `new uint8_t[msgLen]()` without exception guard (leaks on `bad_alloc`).

**Fix:** Add `if (len > 119u) return;` guard; use `uint64_t bitLen`; use `std::vector` instead of manual `new[]`/`delete[]`.

---

### MD-6 — No authentication on UDP CONNECT/DISCONNECT/ACK

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/UDPStream/UDPSServer.cpp:655-723, 729-741` |
| Severity | Medium |
| Type | Missing authentication |

`HandleUnicastConnect` accepts any UDP datagram with the right magic and `type=CONNECT` from any source address. `HandleUnicastDisconnect` evicts any client by source address. `HandleUnicastAck` refreshes any client's `lastSeenTicks`. Any host that can send UDP to the server port can: (a) register as a client and receive all streamed data, (b) disconnect any known client by spoofing their source address (DoS), (c) keep a spoofed client alive indefinitely.

**Fix:** Document the trust boundary; if deployed beyond a trusted LAN, add a shared-secret token to the CONNECT payload and validate source addresses.

---

### MD-7 — `StringHelper::Copy` potential buffer overflow in TcpLogger

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/TCPLogger/TcpLogger.cpp:87` |
| Severity | Medium |
| Type | Potential buffer overflow |

```cpp
StringHelper::Copy(entry.description, description);
```

`entry.description` is `char8[MAX_ERROR_MESSAGE_SIZE]`. If `description` (from `logPage->errorStrBuffer`) is longer than `MAX_ERROR_MESSAGE_SIZE - 1` and `StringHelper::Copy` uses `strcpy` internally, this overflows.

**Fix:** Use `strncpy` with explicit size, or verify `StringHelper::Copy` is bounds-safe.

---

### MD-8 — TcpLogger SPSC queue uses `volatile` indices, no atomics; lost wakeup

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/TCPLogger/TcpLogger.cpp:83-153, 157-158` |
| Severity | Medium |
| Type | Data race / lost wakeup |

`writeIdx`/`readIdx` are `volatile uint32` — no memory ordering on non-x86. `eventSem.Wait(10)` followed by `eventSem.Reset()` is not atomic; a post between `Wait` returning and `Reset` is lost (entry delayed up to 10ms).

**Fix:** Use `Atomic::Load`/`Store`; use `ResetWait` for atomic reset+wait.

---

### MD-9 — `printf`/`fflush` on LoggerService (possibly RT) thread

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/TCPLogger/TcpLogger.cpp:75-76` |
| Severity | Medium |
| Type | RT latency jitter |

`ConsumeLogMessage` is called by `LoggerService`, which may run on an RT thread. `printf`/`fflush` are blocking I/O calls causing latency jitter.

**Fix:** Make stdout mirroring optional via configuration.

---

### MD-10 — No cap on WebSocket client connections (Go hub)

| Field | Value |
|-------|-------|
| File | `Common/Client/go/wshub/hub.go:367-377` |
| Severity | Medium |
| Type | Resource exhaustion |

`HandleWebSocket` unconditionally creates a new `wsClient` with a 64-message buffered channel. No cap on total clients. An attacker can open thousands of connections, exhausting memory and goroutines.

**Fix:** Track `len(h.clients)`; reject new connections above a configurable maximum (e.g. 100).

---

### MD-11 — Silent data loss on all non-blocking channel sends

| Field | Value |
|-------|-------|
| File | `Common/Client/go/wshub/hub.go:346-358, 315-318, 322-326, 330-334, 340-342` |
| Severity | Medium |
| Type | Silent data loss |

`PushDataForSource`, `broadcast`, `AddSource`, `RemoveSource`, `SetSourceState`, `UpdateConfigForSource` all use `select { case ch <- v: default: }`, silently dropping messages when channels are full. Under high data rates, `dataCh` (cap 65536) fills and samples are dropped with no metric or backpressure.

**Fix:** Add a dropped-counter metric; log/alert when drops occur; consider backpressure.

---

### MD-12 — SSRF via unauthenticated `addSource`

| Field | Value |
|-------|-------|
| Files | `Common/Client/go/wshub/hub.go:83-96`; `Common/Client/go/wshub/sources.go:62-67` |
| Severity | Medium |
| Type | Server-Side Request Forgery |

Any WebSocket client (any origin due to CR-3) can add a data source pointing to any `host:port`. The `addr` is passed to `net.ResolveUDPAddr`/`net.ResolveTCPAddr` and the client dials it. This is SSRF — an attacker can probe internal network services.

**Fix:** Validate/allow-list addresses; require authentication for source management.

---

### MD-13 — Reassembler unbounded fragment-set map growth (Go)

| Field | Value |
|-------|-------|
| File | `Common/Client/go/udpsprotocol/reassembler.go:41-89` |
| Severity | Medium |
| Type | Memory exhaustion |

An attacker sending UDP packets with unique `(counter, type)` keys and `TotalFragments=2, FragmentIdx=0` creates a new `fragmentSet` per packet. Each set holds payload bytes and a `make([][]byte, total)` (up to 65535 entries). Within the 2-second TTL, high-rate flooding exhausts memory.

**Fix:** Cap the number of concurrent fragment sets (e.g. `maxSets = 1024`); reject new sets when the cap is reached.

---

### MD-14 — `martecontrol.go` index panic on short "OK SERVICE_INFO" response

| Field | Value |
|-------|-------|
| File | `Client/debugger/martecontrol.go:543` |
| Severity | Medium |
| Type | Panic / crash |

```go
if strings.HasPrefix(line, "OK SERVICE_INFO") {   // matches 15-char string
    ...
    "data": line[len("OK SERVICE_INFO "):],       // line[16:] → panic if len(line)==15
```

**Fix:** Use `strings.TrimPrefix` and handle the empty case, or add a length check.

---

### MD-15 — `pairCount * 16u` overflow on 32-bit (shared C++ client wire layer)

| Field | Value |
|-------|-------|
| File | `Client/streamhub/Protocol.cpp:77, 117` (reused by Qt client) |
| Severity | Medium (32-bit), Low (64-bit) |
| Type | Integer overflow → OOB |

```cpp
uint32_t pairCount = readU32(buf, off, len);
if (off + static_cast<size_t>(pairCount) * 16u > len) { return false; }
```

On 32-bit, `pairCount * 16u` can overflow, bypassing the bounds check. On 64-bit, `pairCount` can be up to ~268M, forcing a large allocation.

**Fix:** Check `pairCount > (len - off) / 16` before multiplication; use `ull` suffix.

---

### MD-16 — `readU16`/`readU32` return 0 on truncation without signaling

| Field | Value |
|-------|-------|
| File | `Client/streamhub/Protocol.cpp:21-38, 66-76` |
| Severity | Medium |
| Type | Silent malformed-frame acceptance |

```cpp
static uint16_t readU16(const uint8_t* buf, size_t& off, size_t len) {
    if (off + 2 > len) { return 0u; }   // returns 0, does NOT signal failure
    ...
}
```

If the buffer is truncated at the `numSignals` field, `readU32` returns 0 and `off` is not advanced. `ParseBinaryFrame` then returns `true` with an empty signal list, silently accepting a truncated/malformed frame as valid.

**Fix:** `readU16`/`readU32`/`readF64` should return a status or set a flag; `ParseBinaryFrame` should fail fast on any truncated read.

---

### MD-17 — JSON command builders use `snprintf`+`%s` with no escaping → JSON injection

| Field | Value |
|-------|-------|
| File | `Client/streamhub/Protocol.cpp:183-186, 209-213` |
| Severity | Medium |
| Type | JSON injection |

```cpp
char buf[256];
snprintf(buf, sizeof(buf), "{\"type\":\"getConfig\",\"sourceId\":\"%s\"}",
         sourceId.c_str());
```

`%s` with user/server-provided strings does not escape JSON special characters. A source label containing `"` breaks the JSON and can inject arbitrary JSON keys. Also `snprintf` truncates silently for long inputs, producing invalid JSON.

**Fix:** Use a JSON builder that escapes strings; use `std::string` instead of fixed buffers.

---

### MD-18 — `strstr`-based JSON parsing matches nested keys

| Field | Value |
|-------|-------|
| File | `Client/streamhub/Protocol.cpp:296-310, 495, 510` |
| Severity | Medium |
| Type | Parser confusion / data corruption |

`ParseSources` uses `strstr(p, "\"id\":\"")` which matches inside string values. `ParseZoom`/`ParseStats` use `strstr(p, "\"t\":[")` which matches inside a signal key name. A crafted source label containing `"id":"` injects phantom sources.

**Fix:** Migrate to a real JSON parser (nlohmann/json for ImGui; QJsonDocument for Qt).

---

### MD-19 — WebSocket RFC 6455 violations in ImGui client

| Field | Value |
|-------|-------|
| File | `Client/streamhub/WSClient.cpp:204-223` |
| Severity | Medium |
| Type | RFC non-compliance |

- No `CONTINUATION` opcode handling (opcode 0x00) — fragmented messages silently dropped.
- No ≤125 byte enforcement on control frames — a malicious server can send a 16MB ping, echoed as a 16MB pong (bandwidth amplification).
- `CLOSE` frame not echoed back (RFC §5.5.1 requires it); close status code not read.

**Fix:** Implement continuation-frame reassembly; reject control frames with `payloadLen > 125`; echo close frame.

---

### MD-20 — Handshake `recv` one byte at a time, no `SO_RCVTIMEO`

| Field | Value |
|-------|-------|
| File | `Client/streamhub/WSClient.cpp:290-301` |
| Severity | Medium |
| Type | Denial of service |

If the server sends a partial response and hangs, the receive thread blocks forever (the 3-second reconnect sleep never runs).

**Fix:** Set `SO_RCVTIMEO` on the socket; read in larger chunks.

---

### MD-21 — 64KB stack buffer in DebugService streamer thread + shadowed member

| Field | Value |
|-------|-------|
| File | `Source/Components/Interfaces/DebugService/DebugService.cpp:438, 489` |
| Severity | Medium |
| Type | Stack pressure |

```cpp
uint8 udpsSampleBuf[UDPS_MAX_SAMPLE_BYTES];   // local 65510 bytes on stack (line 438)
...
static const uint32 CFG_BUF_SIZE = 65535u;
uint8 cfgBuf[CFG_BUF_SIZE];                    // another 64KB on stack (line 489)
```

The class also has a member `uint8 udpsSampleBuf[65535u]` (DebugService.h:156) that is shadowed by the local and never used.

**Fix:** Use the member instead of the local; use heap allocation for `cfgBuf`.

---

### MD-22 — `FastPollingMutexSem` (spinlock) on RT path with network I/O in background holder

| Field | Value |
|-------|-------|
| File | `Source/Components/DataSources/UDPStreamer/UDPStreamer.cpp:856, 947-976` |
| Severity | Medium |
| Type | Priority inversion / unbounded latency |

`Synchronise()` (RT thread) uses `bufMutex.FastLock(TTInfiniteWait)`. The background thread holds `bufMutex` during a `memcpy` of `fill * totalSrcBytes` (potentially large) at line 950-955. If the background thread is preempted while holding the spinlock, the RT thread spins indefinitely, missing its deadline.

**Fix:** Use a non-spinning mutex for the background thread, or ensure the RT-side critical section is minimal (pointer swap, not full memcpy). Consider a triple-buffer.

---

### MD-23 — `configValidated` read without lock

| Field | Value |
|-------|-------|
| File | `Source/Components/DataSources/UDPStreamerClient/UDPStreamerClient.cpp:463` |
| Severity | Medium |
| Type | Data race |

`configValidated` is set under `bufMutex` (line 453) and cleared under `bufMutex` (line 489), but read without the lock at line 463. Technically UB (benign for a `bool` on x86).

**Fix:** Mark `volatile` or acquire the lock before reading.

---

### MD-24 — `parseCapture` panics on truncated input (E2E chain client)

| Field | Value |
|-------|-------|
| File | `Test/E2E/chain/client/main.go:140-171` |
| Severity | Medium |
| Type | Panic / crash |

`parseCapture` reads `keyLen`, `key`, and `n` without checking `off` is within `len(b)`. A truncated v2 frame causes a runtime panic (slice bounds out of range). The only initial check is `len(b) < 29`.

**Fix:** Add bounds checks before each read (`if len(b) < off+2 { return nil, error }`), mirroring `parsePush`.

---

## LOW

### LO-1 — `totalFrags` overflow (caller-controlled)

| File | `Source/Components/Interfaces/UDPStream/UDPSServer.cpp:541-542` |
| Severity | Low |

`payloadSize + maxChunk - 1u` overflows if `payloadSize` is near `UINT32_MAX`. Not network-controlled but a caller bug could trigger it.

---

### LO-2 — `Stop()` TOCTOU on socket deletion

| File | `Source/Applications/StreamHub/WSServer.cpp:104-134` |
| Severity | Low |

`Stop()` calls `sock->Close()` then `Sleep(200ms)` then `delete sock`. The read thread holds a raw `sock` copy. The 200ms sleep may not be sufficient; `delete` could free the socket while the read thread still references it.

**Fix:** Use thread join instead of fixed sleep.

---

### LO-3 — `FastPollingMutexSem` priority inversion on contended non-RT paths

| Files | `UDPSourceSession.h:302, 311, 370`; `WSServer.h:60, 122` |
| Severity | Low |

Spinlocks used for `metaMutex_`, `statsMutex_`, `recSpecMutex_`, `clientsMutex`, `writeMutex`. Priority inversion with no bound if a low-priority holder is preempted by a high-priority spinner.

---

### LO-4 — `SignalBuffer::push` with `capacity == 0` → mod-0 UB

| File | `Client/streamhub/SignalBuffer.h:36-41` |
| Severity | Low |

`head = (head + 1) % capacity` is UB if `capacity == 0`. Not currently reachable (default cap 20000; `onMaxPointsUpdated` guards `mp >= 2`), but the API is fragile.

---

### LO-5 — `SignalBuffer` comment claims "Thread-safe" but has no internal locking

| File | `Client/streamhub/SignalBuffer.h:18` |
| Severity | Low |

Safe only by current call-site discipline (ImGui: main-thread access; Qt: GUI-thread signals). Misleading comment could cause a future refactor to introduce a data race.

---

### LO-6 — SineArrayGAM / TimeArrayGAM: no output-type validation

| Files | `Source/Components/GAMs/SineArrayGAM/SineArrayGAM.cpp:81-100`; `Source/Components/GAMs/TimeArrayGAM/TimeArrayGAM.cpp:54-81` |
| Severity | Low |

`Setup()` never verifies the output signal type. `TimeArrayGAM.h` doc says `uint32` but code uses `uint64` → if user follows the doc and configures `uint32`, `nElements` is doubled and writes overflow the buffer.

**Fix:** Add `GetSignalType(...)` checks; update `TimeArrayGAM.h` doc to `uint64`.

---

### LO-7 — Signal names inserted into JSON via `%s` Printf without escaping

| File | `Source/Components/Interfaces/DebugService/DebugServiceBase.cpp:900-906` |
| Severity | Low |

Config-sourced (not remote), but a format violation. A signal name containing `"` or `\` breaks the JSON.

**Fix:** Use the existing `EscapeJson` helper.

---

### LO-8 — `EvaluateBreak` checks only element 0 of array signals

| File | `Source/Components/Interfaces/DebugService/DebugBrokerWrapper.h:61-86` |
| Severity | Low |

Undocumented feature limitation.

---

### LO-9 — `fprintf(stderr, ...)` on init path

| File | `Source/Components/Interfaces/DebugService/DebugBrokerWrapper.h:195-197` |
| Severity | Low |

Should use `REPORT_ERROR` instead.

---

### LO-10 — `unsafe.Pointer` aliasing in `float64ToBytes`

| File | `Common/Client/go/wshub/hub.go:588-594` |
| Severity | Low |

`unsafe.Slice((*byte)(unsafe.Pointer(&f[0])), len(f)*8)` aliases the float64 backing array. Currently safe (synchronous copy), but fragile if a caller retains the byte slice.

---

### LO-11 — `+Inf` in JSON from division by zero

| File | `Common/Client/go/wshub/stats.go:115-116` |
| Severity | Low |

`si.RateHz = 1.0 / avg` produces `+Inf` if `avg == 0`, yielding invalid JSON.

---

### LO-12 — Directory listing enabled

| File | `Client/webui/main.go:26` |
| Severity | Low |

`http.FileServer(http.Dir(*static))` serves directory listings if a directory lacks `index.html`.

---

### LO-13 — No security headers on static file serving

| File | `Client/debugger/main.go:55` |
| Severity | Low |

No `Content-Security-Policy`, `X-Frame-Options`, `X-Content-Type-Options: nosniff`.

---

### LO-14 — `stopCh` close pattern not `sync.Once`-guarded

| File | `Client/debugger/martecontrol.go:182-189` |
| Severity | Low |

`select { case <-m.stopCh: default: close(m.stopCh) }` is not atomic; concurrent `Disconnect` calls would double-close and panic.

---

### LO-15 — `host_`/`port_` read by recv thread, written by UI thread

| File | `Client/streamhub/WSClient.cpp:48-58, 73-89, 160` |
| Severity | Low |

`std::string` concurrent read/write is technically UB (small window; recv thread re-reads on each reconnect cycle).

---

### LO-16 — `ReadExactTCP` progress edge case

| File | `Source/Components/Interfaces/UDPStream/UDPSClient.cpp:474-487` |
| Severity | Low |

If `Read` returns `true` but leaves `chunk` unchanged (contract violation), `got` would advance without data being read. Depends on MARTe2 `BasicTCPSocket::Read` contract.

---

### LO-17 — `bufMutex.Create(false)` return value unchecked

| Files | `Source/Components/DataSources/UDPStreamer/UDPStreamer.cpp:119`; `Source/Components/DataSources/UDPStreamerClient/UDPStreamerClient.cpp:149` |
| Severity | Low |

If mutex creation fails, all subsequent `FastLock` calls operate on an uninitialized semaphore → UB.

---

### LO-18 — No validation of `RangeMin < RangeMax` for quantization

| File | `Source/Components/DataSources/UDPStreamer/UDPStreamer.cpp:403-404` |
| Severity | Low |

If `rangeMax < rangeMin`, quantization normalization produces negative values clamped to 0.0 → silent data corruption. Divide-by-zero is guarded (`if (rRange == 0.0) rRange = 1.0`).

---

### LO-19 — Reassembler GC ticker panics if `expiry == 0`

| File | `Common/Client/go/udpsprotocol/reassembler.go:93` |
| Severity | Low |

`time.NewTicker(r.expiry / 2)` panics if `r.expiry == 0`. Currently always called with `2 * time.Second`.

---

## Cross-cutting themes

1. **Integer overflow in size arithmetic is pervasive** — every binary protocol parser (C++ producer, C++ consumer, Go decoder, C++ client wire layer) multiplies attacker-controlled counts by element sizes in 32-bit. Uniform fix: validate count against `len(payload)/elementSize` before multiplying; use 64-bit for bounds checks.
2. **No authentication anywhere in the control plane** — DebugService TCP, TcpLogger TCP, both Go web UIs, the C++ StreamHub WS, and the UDPS CONNECT protocol all trust any peer. Bind to localhost by default; document the trust boundary.
3. **WebSocket Origin checks disabled in every WS server** — Go `CheckOrigin: true`; C++ no Origin parse. Drive-by CSRF on all of them.
4. **`volatile` used for cross-thread synchronization** — TraceRingBuffer, TcpLogger queue. Not a memory-ordering primitive on non-x86; use atomics.
5. **`strstr`/`snprintf`-based JSON** in C++ clients — fragile against crafted-but-valid JSON; migrate to a real parser.