Document buffer fast-path preconditions with an assertion

[orbisai0security]

Summary

Retraction and rewrite of the previous version of this PR.

msgpack_buffer_write_byte_and_data has a caller-side precondition: the buffer must have at least length + 1 bytes writable. Every call site in cbor_encoder_write_head (ext/cbor/packer.h:85-98) and the float paths (ext/cbor/packer.h:164-190) already calls msgpack_buffer_ensure_writable with the exact required size on the line above. The precondition is an internal caller contract, not an attacker-reachable bug.

This PR documents that contract with an assert(length + 1 <= msgpack_buffer_writable_size(b)) inside the function. Under NDEBUG the assertion compiles out, so there is zero hot-path cost in release builds; in debug builds it catches the exact invariant violation the original version of this PR was worried about.

It also adds rspec coverage in spec/packer_spec.rb that exercises every cbor_encoder_write_head branch funneling into write_byte_and_data (16-bit, 32-bit, 64-bit heads for unsigned/negative ints, byte/text strings, array/map headers, plus float16/32/64), via round-trip encode/decode.

Retraction

The earlier version of this PR (commits f0dc26f and aa229c2) added a redundant msgpack_buffer_ensure_writable(b, length + 1) call inside msgpack_buffer_write_byte_and_data, and a C test stub (tests/test_invariant_buffer.h) that referenced a non-existent type (cbor_buffer_t), never invoked the function under test, and asserted on a memcpy it executed inside the test body — it passed independent of the library code.

The "CRITICAL / Confirmed exploitable / CWE-120 / V-001" framing was wrong. There is no demonstrated overflow on this code path because every caller pre-sizes the buffer. Apologies for the noise; @cabo's review was right that the right framing is "hardening / invariant documentation," not a security fix, and that the actual untrusted-length surface is the unpacker (SANE_PREALLOCATE and read_raw_body_cont in unpacker.c), not the packer. The CRITICAL/CVE label is withdrawn.

Changes

• ext/cbor/buffer.h — #include <assert.h>; replace the function body's first line with the precondition assertion + a one-line comment naming the caller contract.
• spec/packer_spec.rb — new describe 'head-size branches into write_byte_and_data' context covering every branch by round-trip.
• tests/test_invariant_buffer.h — deleted (broken; the project has no C unit-test harness).

Verification

• bundle exec rake spec should pass — the new specs round-trip every head-size branch.
• The assertion holds for every in-tree call site by construction; if a future caller forgets to pre-size, debug builds will catch it at the point of violation.
• If the project prefers RUBY_ASSERT over plain C assert, happy to adapt.

[cabo]

Please do read the code, in particular what the function msgpack_buffer_ensure_writable might be doing that is always called before msgpack_buffer_write_byte_and_data

[orbisai0security]

Please do read the code, in particular what the function msgpack_buffer_ensure_writable might be doing that is always called before msgpack_buffer_write_byte_and_data

Addressed the comment. Pls review.

[orbisai0security]

✅ Review Feedback Addressed

I've automatically addressed 1 review comment(s):

@cabo's review points out that msgpack_buffer_ensure_writable (called immediately before every msgpack_buffer_write_byte_and_data call) already guarantees the buffer has enough writable space by expanding it if needed. Therefore, the assertion assert(length + 1 <= msgpack_buffer_writable_size(b)) is tautologically true and can never fire — it adds no value as a defensive check. The correct response is to remove the redundant assertion and the #include <assert.h> from ext/cbor/buffer.h. The spec additions in spec/packer_spec.rb provide genuine round-trip test coverage and are kept.

Files modified:

• ext/cbor/buffer.h

The changes have been pushed to this PR branch. Please review!