Files

T

Martino Ferrari 0bea41f866 Implemented better testing and fixed skipepd frames

2026-07-01 16:39:34 +02:00

40 KiB

Raw Blame History

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:

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:

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:

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):

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:

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:

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):

// 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:

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:

// 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:

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:

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):

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):

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 deletes 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:

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:

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:

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:

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:

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)

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

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

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

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

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

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

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)

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

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

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

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

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

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

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

Undocumented feature limitation.

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

Should use REPORT_ERROR instead.

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

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

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

LO-12 — Directory listing enabled

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

LO-13 — No security headers on static file serving

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

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

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

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

LO-16 — `ReadExactTCP` progress edge case

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

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

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

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`

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

Cross-cutting themes

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.
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.
WebSocket Origin checks disabled in every WS server — Go CheckOrigin: true; C++ no Origin parse. Drive-by CSRF on all of them.
volatile used for cross-thread synchronization — TraceRingBuffer, TcpLogger queue. Not a memory-ordering primitive on non-x86; use atomics.
strstr/snprintf-based JSON in C++ clients — fragile against crafted-but-valid JSON; migrate to a real parser.

40 KiB Raw Blame History

Bug Report — Security & Correctness Audit

Severity scale

CRITICAL

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

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

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

CR-4 — Unauthenticated arbitrary command injection to MARTe2

CR-5 — No authentication on DebugService TCP command interface

HIGH

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

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

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

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

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

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

HI-7 — Weak PRNG for WebSocket handshake key

HI-8 — Global registry patching of all brokers

HI-9 — TraceRingBuffer not thread-safe

MEDIUM

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

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

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

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

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

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

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

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

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

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

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

MD-12 — SSRF via unauthenticated addSource

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

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

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

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

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

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

MD-19 — WebSocket RFC 6455 violations in ImGui client

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

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

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

MD-23 — configValidated read without lock

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

LOW

LO-1 — totalFrags overflow (caller-controlled)

LO-2 — Stop() TOCTOU on socket deletion

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

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

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

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

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

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

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

LO-10 — unsafe.Pointer aliasing in float64ToBytes

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

LO-12 — Directory listing enabled

LO-13 — No security headers on static file serving

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

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

LO-16 — ReadExactTCP progress edge case

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

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

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

Cross-cutting themes

40 KiB

Raw Blame History

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

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

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

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

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

HI-9 — `TraceRingBuffer` not thread-safe

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

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

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

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

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

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

MD-12 — SSRF via unauthenticated `addSource`

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

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

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

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

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

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

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

MD-23 — `configValidated` read without lock

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

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

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

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

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

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

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

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

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

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

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

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

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

LO-16 — `ReadExactTCP` progress edge case

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

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

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