53 lines
2.5 KiB
Markdown
53 lines
2.5 KiB
Markdown
# 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:
|
|
|
|
```powershell
|
|
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.
|