Files
MAG160C/.superpowers/sdd/task-2-fix-1-brief.md

2.5 KiB

Task 2 Fix Brief: oversized TCM payload safety

Finding to fix

Task 2 reviewer found an Important issue:

  • In src/core/tcm_frame.cpp, payload.size() + 5U is cast to uint16_t. For payloads larger than 65530 bytes, the body length truncates, the vector is allocated too small, and the copy loop writes past the encoded vector.
  • In src/c_api.cpp, mag160c_tcm_encode_frame encodes before checking output buffer, so even size-query calls with out_bytes == nullptr can trigger the overflow.

Required fix

  • Prevent oversized payloads from corrupting memory.
  • Maximum payload size is 65530 bytes because body length is uint16_t and body length is payload_size + 5.
  • C++ encode_tcm_frame must reject oversized payloads before allocation/copy.
  • C ABI mag160c_tcm_encode_frame must return MAG160C_ERR_INVALID_ARGUMENT for oversized payloads and set last_error explaining the payload is too large.
  • Add tests covering oversized payload handling.

Suggested implementation

  • In src/core/tcm_frame.cpp, add a helper constant or check:
    • constexpr size_t MAX_TCM_PAYLOAD_SIZE = 0xffffU - 5U;
    • if payload.size() > MAX_TCM_PAYLOAD_SIZE, throw std::length_error("TCM payload is too large");
  • In src/c_api.cpp, validate payload_size before constructing payload_vec or calling encode_tcm_frame.
  • Optionally catch std::length_error around encode_tcm_frame and map to MAG160C_ERR_INVALID_ARGUMENT for defense-in-depth.

Required tests

Add at least one no-crash test:

  • In tests/cpp/test_c_api.cpp, allocate a payload of size 65531 and call mag160c_tcm_encode_frame with out_bytes nullptr and capacity 0.
  • Assert it returns MAG160C_ERR_INVALID_ARGUMENT.
  • Assert out_size is 0.
  • Assert mag160c_last_error() contains "too large".

Also add a core test if convenient:

  • In tests/cpp/test_tcm_frame.cpp, call encode_tcm_frame with payload size 65531 and assert it throws std::length_error.

Verification

Run direct verification:

g++ -std=c++17 -DMAG160C_STATIC -DMAG160C_HAS_LIBUSB=0 -Iinclude -Isrc src\core\error.cpp src\core\context.cpp src\core\tcm_frame.cpp src\c_api.cpp tests\cpp\test_c_api.cpp -o build_task2_c_api_test.exe
.\build_task2_c_api_test.exe
g++ -std=c++17 -DMAG160C_STATIC -DMAG160C_HAS_LIBUSB=0 -Iinclude -Isrc src\core\error.cpp src\core\context.cpp src\core\tcm_frame.cpp src\c_api.cpp tests\cpp\test_tcm_frame.cpp -o build_task2_tcm_frame_test.exe
.\build_task2_tcm_frame_test.exe

Report contract

Append a fix report section to .superpowers/sdd/task-2-report.md and return status plus one-line test summary.