Skip to content

Commit 28ba1e3

Browse files
Watson1978claude
andcommitted
Fix use-after-free when packing a string into an IO
msgpack_buffer_append_string read the string's raw pointer up front and then grew the buffer. Growing it runs io.write (arbitrary Ruby) and can trigger GC, either of which may free or shrink the string's backing memory, so the later memcpy could read freed memory and copy heap contents into the output. Reserve the space first and read the pointer only afterwards, clamping and zero-filling if the string shrank in the meantime so the copy stays within the current buffer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 314d65d commit 28ba1e3

1 file changed

Lines changed: 19 additions & 4 deletions

File tree

‎ext/msgpack/buffer.h‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,14 +237,29 @@ void _msgpack_buffer_append_long_string(msgpack_buffer_t* b, VALUE string);
237237

238238
static inline size_t msgpack_buffer_append_string(msgpack_buffer_t* b, VALUE string)
239239
{
240-
size_t length;
241-
char *ptr;
242-
RSTRING_GETMEM(string, ptr, length);
240+
size_t length = RSTRING_LEN(string);
243241

244242
if(length > b->write_reference_threshold) {
245243
_msgpack_buffer_append_long_string(b, string);
246244
} else {
247-
msgpack_buffer_append(b, ptr, length);
245+
/* Making room can run io.write or trigger GC, either of which may free
246+
* or shrink the string, so reserve the space first and only then read
247+
* its pointer. */
248+
if(length > msgpack_buffer_writable_size(b)) {
249+
msgpack_buffer_ensure_writable(b, length);
250+
}
251+
char *ptr;
252+
size_t current;
253+
RSTRING_GETMEM(string, ptr, current);
254+
if(current > length) {
255+
current = length;
256+
}
257+
memcpy(b->tail.last, ptr, current);
258+
if(current < length) {
259+
memset(b->tail.last + current, 0, length - current);
260+
}
261+
b->tail.last += length;
262+
RB_GC_GUARD(string);
248263
}
249264

250265
return length;

0 commit comments

Comments
 (0)