gui: fix cJSON_Parse overrun

Author: jherrera-jump

src/app/firedancer/topology.c

@@ -428,7 +428,7 @@ fd_topo_initialize( config_t * config ) {
     /**/               fd_topob_link( topo, "snapct_repr",  "snapct_repr",  128UL,                                    0UL,                           1UL )->permit_no_consumers = 1; /* TODO: wire in repair later */
     if( FD_LIKELY( config->tiles.gui.enabled ) ) {
       /**/             fd_topob_link( topo, "snapct_gui",   "snapct_gui",   128UL,                                    sizeof(fd_snapct_update_t),    1UL );
-      /**/             fd_topob_link( topo, "snapin_gui",   "snapin_gui",   128UL,                                    FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ, 1UL );
+      /**/             fd_topob_link( topo, "snapin_gui",   "snapin_gui",   128UL,                                    FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ_WITH_NULL, 1UL );
     }
     if( vinyl_enabled ) {
       fd_topo_link_t * snapin_wh =

src/disco/gui/fd_gui_config_parse.c

@@ -66,7 +66,7 @@ fd_gui_config_parse_validator_info_check( uchar const * data,
 
   CHECK_LEFT( sizeof(ulong) ); ulong json_str_sz = FD_LOAD( ulong, data+i ); i += sizeof(ulong);
 
-  CHECK_LEFT( json_str_sz );
+  CHECK_LEFT( json_str_sz+1UL ); /* cJSON_ParseWithLengthOpts requires having byte after the JSON payload */
   cJSON * json = cJSON_ParseWithLengthOpts( (char *)(data+i), json_str_sz, NULL, 0 );
   if( FD_UNLIKELY( !json ) ) return 0;
 

src/disco/gui/fd_gui_config_parse.h

@@ -6,13 +6,14 @@
 #include "../../util/fd_util_base.h"
 
 /* https://github.com/anza-xyz/agave/blob/master/account-decoder/src/validator_info.rs */
-#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_NAME_SZ     (  80UL) /* +1UL for NULL terminator */
-#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_WEBSITE_SZ  (  80UL)
-#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_DETAILS_SZ  ( 300UL)
-#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_ICON_URI_SZ (  80UL)
-#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_KEYBASE_USERNAME_SZ (80UL)
-#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_MAX_SZ      ( 576UL) /* does not include size of ConfigKeys */
-#define FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ          (FD_GUI_CONFIG_PARSE_CONFIG_KEYS_MAX_SZ+FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_MAX_SZ)
+#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_NAME_SZ             (  80UL) /* +1UL for NULL terminator */
+#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_WEBSITE_SZ          (  80UL)
+#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_DETAILS_SZ          ( 300UL)
+#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_ICON_URI_SZ         (  80UL)
+#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_KEYBASE_USERNAME_SZ (  80UL)
+#define FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_MAX_SZ              ( 576UL) /* does not include size of ConfigKeys */
+#define FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ                  (FD_GUI_CONFIG_PARSE_CONFIG_KEYS_MAX_SZ+FD_GUI_CONFIG_PARSE_VALIDATOR_INFO_MAX_SZ)
+#define FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ_WITH_NULL        (FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ+1UL) /* cJSON parser requires one byte past the parsable JSON */
 
 /* The size of a ConfigKeys of length 2, which is the expected length of ValidatorInfo */
 #define FD_GUI_CONFIG_PARSE_CONFIG_KEYS_MAX_SZ         (1UL + (32UL + 1UL)*2UL)

src/disco/gui/fuzz_config_parser.c

@@ -27,17 +27,16 @@ LLVMFuzzerTestOneInput( uchar const * data,
   cJSON * json;
   fd_gui_config_parse_info_t validator_info[1];
   fd_pubkey_t pubkey;
-  FD_LOG_WARNING(("TEST"));
   int valid = fd_gui_config_parse_validator_info_check( data, size, &json, &pubkey );
 
   if( valid ) {
     fd_gui_config_parse_validator_info( json, validator_info );
 
-    assert( fd_utf8_verify( validator_info->name,             strlen( validator_info->name ) ) );
-    assert( fd_utf8_verify( validator_info->website,          strlen( validator_info->name ) ) );
-    assert( fd_utf8_verify( validator_info->details,          strlen( validator_info->name ) ) );
-    assert( fd_utf8_verify( validator_info->icon_uri,         strlen( validator_info->name ) ) );
-    assert( fd_utf8_verify( validator_info->keybase_username, strlen( validator_info->name ) ) );
+    assert( fd_utf8_verify( validator_info->name,             strlen( validator_info->name )             ) );
+    assert( fd_utf8_verify( validator_info->website,          strlen( validator_info->website )          ) );
+    assert( fd_utf8_verify( validator_info->details,          strlen( validator_info->details )          ) );
+    assert( fd_utf8_verify( validator_info->icon_uri,         strlen( validator_info->icon_uri )         ) );
+    assert( fd_utf8_verify( validator_info->keybase_username, strlen( validator_info->keybase_username ) ) );
   }
   return 0;
 }

src/discof/restore/fd_snapin_tile.c

@@ -519,16 +519,19 @@ handle_data_frag( fd_snapin_tile_t *  ctx,
 
         /* We exepect ConfigKeys Vec to be length 2.  We expect the size
            of ConfigProgram-owned accounts to be
-           FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ, since this the the
-           size that the solana CLI allocates for them.  Although the
-           Config program itself does not enforce this limit, the vast
-           majority of accounts (with a tiny number of excpetions on
-           devnet) are maintained with the solana cli. */
+           FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ, since this the size
+           that the solana CLI allocates for them.  Although the Config
+           program itself does not enforce this limit, the vast majority
+           of accounts (with a tiny number of excpetions on devnet) are
+           maintained with the solana cli. */
         if( FD_UNLIKELY( ctx->gui_out.idx!=ULONG_MAX && !memcmp( result->account_data.owner, fd_solana_config_program_id.key, sizeof(fd_hash_t) ) && result->account_data.data_sz && *(uchar *)result->account_data.data==2UL && result->account_data.data_sz<=FD_GUI_CONFIG_PARSE_MAX_VALID_ACCT_SZ ) ) {
           uchar * acct = fd_chunk_to_laddr( ctx->gui_out.mem, ctx->gui_out.chunk );
           fd_memcpy( acct, result->account_data.data, result->account_data.data_sz );
-          fd_stem_publish( stem, ctx->gui_out.idx, 0UL, ctx->gui_out.chunk, result->account_data.data_sz, 0UL, 0UL, 0UL );
-          ctx->gui_out.chunk = fd_dcache_compact_next( ctx->gui_out.chunk, result->account_data.data_sz, ctx->gui_out.chunk0, ctx->gui_out.wmark );
+
+          /* We add 1 to the frag size since the cJSON parser used by
+             the gui tile requires one byte past the parsable JSON */
+          fd_stem_publish( stem, ctx->gui_out.idx, 0UL, ctx->gui_out.chunk, result->account_data.data_sz+1UL, 0UL, 0UL, 0UL );
+          ctx->gui_out.chunk = fd_dcache_compact_next( ctx->gui_out.chunk, result->account_data.data_sz+1UL, ctx->gui_out.chunk0, ctx->gui_out.wmark );
         }
         break;
       case FD_SSPARSE_ADVANCE_ACCOUNT_BATCH: