Skip to content

Fix ext32 unpacking of the maximum UINT32_MAX-byte payload #1184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
afonsojanu wants to merge 1 commit into
base: c_master
Choose a base branch
from
Open

Fix ext32 unpacking of the maximum UINT32_MAX-byte payload #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions include/msgpack/unpack_template.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ msgpack_unpack_struct_decl(_stack) {
msgpack_unpack_struct_decl(_context) {
msgpack_unpack_user user;
unsigned int cs;
unsigned int trail;
size_t trail;
unsigned int top;
/*
msgpack_unpack_struct(_stack)* stack;
Expand Down Expand Up @@ -99,7 +99,7 @@ msgpack_unpack_func(int, _execute)(msgpack_unpack_struct(_context)* ctx, const c
const unsigned char* const pe = (unsigned char*)data + len;
const void* n = NULL;

unsigned int trail = ctx->trail;
size_t trail = ctx->trail;
unsigned int cs = ctx->cs;
unsigned int top = ctx->top;
msgpack_unpack_struct(_stack)* stack = ctx->stack;
Expand Down Expand Up @@ -326,7 +326,7 @@ msgpack_unpack_func(int, _execute)(msgpack_unpack_struct(_context)* ctx, const c
case MSGPACK_CS_EXT_16:{
uint16_t tmp;
_msgpack_load16(uint16_t,n,&tmp);
again_fixed_trail_if_zero(MSGPACK_ACS_EXT_VALUE, tmp + 1, _ext_zero);
again_fixed_trail_if_zero(MSGPACK_ACS_EXT_VALUE, (size_t)tmp + 1, _ext_zero);
}
case MSGPACK_CS_STR_32:{
uint32_t tmp;
Expand All @@ -341,7 +341,11 @@ msgpack_unpack_func(int, _execute)(msgpack_unpack_struct(_context)* ctx, const c
case MSGPACK_CS_EXT_32:{
uint32_t tmp;
_msgpack_load32(uint32_t,n,&tmp);
again_fixed_trail_if_zero(MSGPACK_ACS_EXT_VALUE, tmp + 1, _ext_zero);
/* cast before adding: on a 64-bit size_t this lets an ext32 with
* the maximum UINT32_MAX-byte payload carry its trailing type
* byte without wrapping trail back to 0 the way `tmp + 1` would
* when tmp is exactly UINT32_MAX */
again_fixed_trail_if_zero(MSGPACK_ACS_EXT_VALUE, (size_t)tmp + 1, _ext_zero);
}
case MSGPACK_ACS_STR_VALUE:
_str_zero:
Expand Down
6 changes: 3 additions & 3 deletions src/unpack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ static inline int template_callback_map_item(unpack_user* u, msgpack_object* c,
return 0;
}

static inline int template_callback_str(unpack_user* u, const char* b, const char* p, unsigned int l, msgpack_object* o)
static inline int template_callback_str(unpack_user* u, const char* b, const char* p, size_t l, msgpack_object* o)
{
MSGPACK_UNUSED(b);
if (*u->z == NULL) {
Expand All @@ -291,7 +291,7 @@ static inline int template_callback_str(unpack_user* u, const char* b, const cha
return 0;
}

static inline int template_callback_bin(unpack_user* u, const char* b, const char* p, unsigned int l, msgpack_object* o)
static inline int template_callback_bin(unpack_user* u, const char* b, const char* p, size_t l, msgpack_object* o)
{
MSGPACK_UNUSED(b);
if (*u->z == NULL) {
Expand All @@ -307,7 +307,7 @@ static inline int template_callback_bin(unpack_user* u, const char* b, const cha
return 0;
}

static inline int template_callback_ext(unpack_user* u, const char* b, const char* p, unsigned int l, msgpack_object* o)
static inline int template_callback_ext(unpack_user* u, const char* b, const char* p, size_t l, msgpack_object* o)
{
MSGPACK_UNUSED(b);
if (l == 0) {
Expand Down
34 changes: 34 additions & 0 deletions test/msgpack_c.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,40 @@ TEST(MSGPACKC, simple_buffer_fixext_4byte_65536)
msgpack_sbuffer_destroy(&sbuf);
}

TEST(MSGPACKC, simple_buffer_ext_maxlen)
{
// ext32's length header covers up to UINT32_MAX bytes of data, so this
// needs roughly 8GB of free memory (the source buffer plus the packed
// copy) and will skip itself rather than fail on a host that doesn't
// have that much to spare.
const size_t size = static_cast<size_t>(UINT32_MAX);
void* buf = calloc(size, 1);
if (buf == NULL) {
GTEST_SKIP() << "not enough memory to allocate a " << size << " byte buffer";
}

msgpack_sbuffer sbuf;
msgpack_sbuffer_init(&sbuf);
msgpack_packer pk;
msgpack_packer_init(&pk, &sbuf, msgpack_sbuffer_write);

msgpack_pack_ext(&pk, size, 82);
msgpack_pack_ext_body(&pk, buf, size);
msgpack_zone z;
msgpack_zone_init(&z, 2048);
msgpack_object obj;
msgpack_unpack_return ret =
msgpack_unpack(sbuf.data, sbuf.size, NULL, &z, &obj);
EXPECT_EQ(MSGPACK_UNPACK_SUCCESS, ret);
EXPECT_EQ(MSGPACK_OBJECT_EXT, obj.type);
EXPECT_EQ(82, obj.via.ext.type);
ASSERT_EQ(size, obj.via.ext.size);
EXPECT_EQ(0, memcmp(buf, obj.via.ext.ptr, size));
msgpack_zone_destroy(&z);
msgpack_sbuffer_destroy(&sbuf);
free(buf);
}

TEST(MSGPACKC, simple_buffer_timestamp_32)
{
msgpack_timestamp ts = {
Expand Down
Loading