fix buffer overflow errors

[QVPham9601]

This fixes use the new variable "remaining_len" to control the real remaining length of the buffer in the function _convert_execution_trace_to_json_string

Closes #16

Decision Record

Root cause: _convert_execution_trace_to_json_string used a single total_len counter (decremented as writes happened) rather than computing remaining capacity from the current pointer position. On event boundaries this could advance str_ptr one byte past buffer and produce a size_t underflow in the next remaining_len computation, passing a huge length to snprintf.

Options considered:

1. Keep total_len but fix the off-by-one arithmetic at each boundary.
2. Replace total_len with a per-write remaining_len = sizeof(buffer) - (str_ptr - buffer) - 1 computation.
3. Introduce a safe-write helper that encapsulates the bounds check.

Options rejected:

• Option 1: the accumulated-decrement approach is fragile; any future write that skips the decrement reintroduces the bug.
• Option 3: adds indirection without simplifying the callers meaningfully at this scale.

Selected option: Option 2 — recompute remaining_len from pointer arithmetic before every snprintf or direct byte write. Track per-event start with event_start; on any truncation revert str_ptr to event_start and stop, guaranteeing the returned JSON is always well-formed.

Residual risk: inet_ntop failure (unusual in practice) now safely aborts the current event rather than emitting a valueless ["proto.att",] entry. The _copy_plein_text limit is intentionally capped at remaining_len - 20 to leave headroom for the surrounding JSON delimiters.

Acceptance Criteria Verification

#	Criterion	Status	Evidence
1	str_ptr never advances beyond the bounds of buffer, including event boundaries	pass	event_start captures position before each event; every advance is gated on a remaining_len check; truncation reverts to event_start (mmt_security.c:459,675)
2	Function exits safely without OOB write or size_t underflow when space is insufficient	pass	remaining_len recomputed from sizeof(buffer) - (str_ptr - buffer) - 1 before each write; remaining_len == 0 and size >= remaining_len checks guard all paths (mmt_security.c:463–482)
3	Truncation paths do not produce malformed JSON	pass	Truncated events are dropped entirely (revert to event_start); inet_ntop failure now sets truncated=YES rather than writing an empty value, preventing ["proto.att",] output (mmt_security.c:570–574)

[Copilot]

Pull request overview

Updates _convert_execution_trace_to_json_string to compute a per-write remaining buffer length (remaining_len) and use it as the size limit for several snprintf calls, aiming to prevent buffer overflows while building a JSON string representation of an execution trace.

Changes:

• Introduce remaining_len derived from buffer size and the current str_ptr position.
• Replace several snprintf(..., total_len, ...) calls with snprintf(..., remaining_len, ...) to better reflect actual remaining space.
• Apply the new remaining-length computation in multiple formatting paths (event header, attribute header, numeric formatting, several DPI type formats).

Comments suppressed due to low confidence (1)

src/lib/mmt_security.c:472

• remaining_len is used as the snprintf limit, but the code later treats size as “bytes written” (it’s used to update str_ptr / total_len). snprintf returns the number of chars that would have been written (or a negative value on error), so when truncation happens size >= remaining_len and subsequent pointer arithmetic can jump past buffer and reintroduce out-of-bounds writes. Handle snprintf’s return value explicitly (error vs truncated vs fully written) and only advance str_ptr by the actual appended length (typically min(ret, remaining_len-1)).

		remaining_len = sizeof(buffer) - (str_ptr - buffer) - 1;
		size = snprintf( str_ptr, remaining_len, "\"event_%zu\":{\"timestamp\":%ld.%06ld,\"counter\":%"PRIu64",\"attributes\":[",
						index,
						time.tv_sec, //timestamp: second
						time.tv_usec, //timestamp: microsecond
						msg->counter );

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

After introducing remaining_len, the function now has two independent “remaining length” concepts (remaining_len and total_len). total_len is still used later for bounds checks and for sizing other writes, but it is not kept in sync with manual pointer increments (e.g., writing separators/brackets via *(str_ptr++) = ...). This desynchronization can defeat the intended overflow fix. Consider removing total_len entirely and consistently computing remaining space from str_ptr, or ensure every manual write decrements total_len so both stay consistent.

[Copilot]

remaining_len is computed once for the dpi_type switch, but not all branches honor it: the IPv6 branch below still uses sprintf (no bounds), and it also leaves size unchanged if inet_ntop fails, which can make subsequent str_ptr += size use a stale value. Use a bounded write (snprintf with the current remaining length) and set size deterministically (e.g., 0 on failure) in every branch.

[Copilot]

This snprintf has the same truncation/return-value issue: if remaining_len is too small, snprintf can return a value >= remaining_len, but the code immediately does str_ptr += size and total_len -= size. That can move str_ptr beyond buffer even though only remaining_len-1 bytes were actually written. Please clamp size to the actual appended length (and handle negative returns) before using it for pointer/length updates.

[Copilot]

Same snprintf return-value handling problem here: size is derived from snprintf and later used to advance str_ptr and write the closing bracket. If formatting is truncated (or snprintf errors), size won’t equal the number of bytes actually placed in buffer, and this can still produce out-of-bounds writes. Please validate/clamp the return value before using size for pointer arithmetic.

[luongnv89]

Nice cleanup overall — the event-boundary rollback and latest inet_ntop failure handling make this PR look safe to merge.

One non-blocking follow-up note: numeric serialization still uses snprintf("%.2f", double_val). If double_val is ever NaN or ±Inf, the function may emit nan / inf / -inf, which are not valid JSON numbers. I don't think this should block merge for the current overflow fix, but it would be worth handling in a follow-up if those values can reach this path.