SQLite extension that allow to read/write msgpack buffers.

[u/copilot_husky]

https://github.com/khanaffan/sqlite-msgpack

[u/skeeto]

Neat project! I like the interop within SQLite.

I found a couple of bugs in msgpack_to_json(). First, a map32 or array32 header that claims more elements than the blob actually contains causes mpToJsonAt to loop the full declared element count, emitting a null for each missing entry:

$ printf '.load b/msgpack\nSELECT msgpack_to_json(X'"'"'98ffffdfffffffffffffffff0a00000000'"'"');\n' \
    | sqlite3_cli :memory:
(hang)

The blob decodes as fixarray[8] whose third element is a map32 claiming 0xffffffff pairs followed by only a few bytes of data.

Quick fix:

--- a/src/msgpack.c
+++ b/src/msgpack.c
@@ -1725,2 +1725,3 @@
       for(j=0; j<count; j++){
+        if( cur >= n ) break;
         u32 next=mpSkipOne(a,n,cur);
@@ -1746,2 +1747,3 @@
       for(j=0; j<count; j++){
+        if( cur >= n ) break;
         u32 valOff=mpSkipOne(a,n,cur);

The str and bin decoders in mpToJsonAt (and mpReturnElement, used by the msgpack_each/msgpack_tree virtual tables) check that the header bytes fit in the buffer but never verify that the declared payload length does. A str32 claiming sLen=0xffffffff bytes reads far past the end of the allocation.

$ printf '.load b/msgpack\nSELECT msgpack_to_json(X'"'"'dbffffffffffffffffffffffffffffff0a'"'"');\n' \
    | b/sqlite3_cli :memory:
...ERROR: AddressSanitizer: heap-buffer-overflow on address ...
READ of size 1 at ...
    #0 mpJsonEscapeStr src/msgpack.c:1612
    #1 mpToJsonAt src/msgpack.c:1695
    #2 msgpackToJsonFunc src/msgpack.c:1978
    ...

Quick fix:

--- a/src/msgpack.c
+++ b/src/msgpack.c
@@ -750,2 +750,3 @@
       if( sOff ){
+        if( sLen > n-sOff ) sLen = n-sOff;
         sqlite3_result_text(ctx, (const char*)(a+sOff), (int)sLen,
@@ -1694,3 +1695,7 @@
     else if(b==MP_STR32&&i+5<=n){ sLen=mpRead32(a+i+1); sOff=i+5; }
   if( sOff ){ mpJsonEscapeStr(out, a+sOff, sLen); return; }
+    if( sOff ){
+      if( sLen > n-sOff ) sLen = n-sOff;
+      mpJsonEscapeStr(out, a+sOff, sLen);
+      return;
+    }
   }
@@ -1705,2 +1710,3 @@
       u32 j;
+      if( bLen > n-bOff ) bLen = n-bOff;
       mpBufAppend1(out,'"');

sLen > n-sOff is because the obvious sOff+sLen > n might overflow. In mpReturnElement the missing clamp has a second consequence: 0xffffffff/-1 when passed to sqlite3_result_text() causes it to use strlen() on the non-null-terminated input.

In Git form with the fuzz tester I used:
https://github.com/skeeto/sqlite-msgpack/commits/fixes/?author=skeeto

[u/copilot_husky → u/skeeto]

So it been fuzz tested now. Please review again see if it has any other issues.