From 64ff9da70f2017fbe7b0d4e18765044579e85085 Mon Sep 17 00:00:00 2001 From: Affan Khan Date: Thu, 2 Jul 2026 12:03:20 -0400 Subject: [PATCH] Fix heap overflow in mpMergePatch via malicious map count An MP_MAP32 header can declare a pair count up to 0x80000001. This value was multiplied by sizeof(MpPatchEntry)/sizeof(int) and passed to sqlite3_malloc(int), which truncates 64-bit sizes to a signed 32-bit int, causing a tiny under-allocation. The subsequent loop then wrote pCount entries into the undersized buffer, producing a heap-buffer-overflow reachable via msgpack_patch() on untrusted msgpack blobs (found via libFuzzer + ASan). Fixes: - Reject patch map headers whose declared pair count cannot fit in the remaining buffer (every pair needs >=2 bytes), rejecting malformed/truncated input before any allocation. - Switch the pIdx and phash allocations from sqlite3_malloc(int) to sqlite3_malloc64(sqlite3_uint64), removing the signed-int truncation entirely. Adds regression checks in test_spec_p5_mutation.c (6.10-6.12) for bogus MAP32/MAP16 counts and a truncated MAP32 header, plus the minimized crash input as a new fuzz corpus seed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/msgpack.c | 11 +++++- .../ce9d9941f60c826606da11f619193d9554a677ab | Bin 0 -> 8 bytes tests/test_spec_p5_mutation.c | 37 ++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/fuzz_corpus/ce9d9941f60c826606da11f619193d9554a677ab diff --git a/src/msgpack.c b/src/msgpack.c index 898925c..b077eb1 100644 --- a/src/msgpack.c +++ b/src/msgpack.c @@ -1733,6 +1733,13 @@ static int mpMergePatch( pCount=mpRead32(p+ip+1); pDataOff=ip+5; } + /* Every map pair needs at least 2 bytes (>=1-byte key + >=1-byte value), so a + ** valid map cannot declare more than (np-pDataOff)/2 pairs. Reject counts + ** that cannot fit in the remaining buffer. Besides rejecting malformed or + ** truncated input early, this bounds pCount so the index allocations below + ** cannot overflow. (pDataOff<=np is guaranteed above.) */ + if( pCount > (np - pDataOff)/2 ) return SQLITE_ERROR; + u32 aCount=0, aDataOff=0; if(aIsMap){ if(ab>=0x80&&ab<=0x8f) { aCount=ab&0x0f; aDataOff=ia+1; } @@ -1749,7 +1756,7 @@ static int mpMergePatch( MpPatchEntry pStack[16]; MpPatchEntry *pIdx = pStack; if( pCount > 16 ){ - pIdx = (MpPatchEntry*)sqlite3_malloc(pCount * sizeof(MpPatchEntry)); + pIdx = (MpPatchEntry*)sqlite3_malloc64((sqlite3_uint64)pCount * sizeof(MpPatchEntry)); if( !pIdx ) return SQLITE_NOMEM; } { u32 k, pc2 = pDataOff; @@ -1780,7 +1787,7 @@ static int mpMergePatch( int phHeap = 0; while( phSize < pCount*2 ) phSize <<= 1; if( phSize > 64 ){ - phash = (int*)sqlite3_malloc((int)(sizeof(int)*phSize)); + phash = (int*)sqlite3_malloc64((sqlite3_uint64)sizeof(int)*phSize); if( !phash ){ mpBufReset(&tmp); if(pIdx!=pStack) sqlite3_free(pIdx); return SQLITE_NOMEM; } phHeap = 1; } diff --git a/tests/fuzz_corpus/ce9d9941f60c826606da11f619193d9554a677ab b/tests/fuzz_corpus/ce9d9941f60c826606da11f619193d9554a677ab new file mode 100644 index 0000000000000000000000000000000000000000..022c2b6be855967b5e42dc525e4f3839b5866c39 GIT binary patch literal 8 PcmccLz`($0$jAf$3mXBI literal 0 HcmV?d00001 diff --git a/tests/test_spec_p5_mutation.c b/tests/test_spec_p5_mutation.c index c05e12d..c12b4f3 100644 --- a/tests/test_spec_p5_mutation.c +++ b/tests/test_spec_p5_mutation.c @@ -172,6 +172,20 @@ static unsigned char *apply_patch(sqlite3 *db, sqlite3_finalize(s); return r; } +/* Run msgpack_patch(target,patch) with raw (possibly malformed) blobs and +** return the sqlite3_step() result code. Used to prove that malformed patches +** are rejected gracefully (SQLITE_ERROR) instead of crashing. */ +static int patch_status(sqlite3 *db, + const void *target, int ntarget, + const void *patch, int npatch){ + sqlite3_stmt *s = NULL; int rc; + sqlite3_prepare_v2(db, "SELECT msgpack_patch(?,?)", -1, &s, NULL); + sqlite3_bind_blob(s, 1, target, ntarget, SQLITE_STATIC); + sqlite3_bind_blob(s, 2, patch, npatch, SQLITE_STATIC); + rc = sqlite3_step(s); + sqlite3_finalize(s); return rc; +} + #ifdef SQLITE_CORE int sqlite3_msgpack_init(sqlite3*, char**, const sqlite3_api_routines*); #endif @@ -509,6 +523,29 @@ static void test_patch(sqlite3 *db){ " '$.b')"); CHECK("6.9 patch adds new key $.b=2 (SQL text)", r && strcmp(r,"2")==0); sqlite3_free(r); } + + /* Regression (fuzz): a map header that declares far more pairs than the + ** buffer can hold must be rejected, not crash. Previously an MP_MAP32 count + ** such as 0x80000001 overflowed the signed-int index allocation, causing a + ** heap-buffer-overflow in mpMergePatch. */ + { + unsigned char target[] = { 0x80 }; /* empty fixmap {} */ + + /* MP_MAP32 declaring 0x80000001 pairs, only a couple of bytes of body. */ + unsigned char bad32[] = { 0xdf, 0x80, 0x00, 0x00, 0x01, 0x31, 0x01, 0x02 }; + CHECK("6.10 patch: bogus MAP32 count rejected (no overflow)", + patch_status(db, target, (int)sizeof target, bad32, (int)sizeof bad32) == SQLITE_ERROR); + + /* MP_MAP16 declaring 0xFFFF pairs with an almost-empty body. */ + unsigned char bad16[] = { 0xde, 0xff, 0xff, 0xa1, 0x6b, 0x01 }; + CHECK("6.11 patch: bogus MAP16 count rejected (no overflow)", + patch_status(db, target, (int)sizeof target, bad16, (int)sizeof bad16) == SQLITE_ERROR); + + /* Truncated MP_MAP32 header (fewer than 5 bytes) must also be rejected. */ + unsigned char trunc[] = { 0xdf, 0xff, 0xff }; + CHECK("6.12 patch: truncated MAP32 header rejected", + patch_status(db, target, (int)sizeof target, trunc, (int)sizeof trunc) == SQLITE_ERROR); + } } /* ── immutability / original unmodified ─────────────────────────── */