Skip to content

Commit

Permalink
shred: add extra shred validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@ptaffet-jump @mmcgee-jump
ptaffet-jump authored and mmcgee-jump committed Sep 19, 2024
1 parent 352c281 commit 9d12a76
Show file tree
Hide file tree
Showing 9 changed files with 216 additions and 28 deletions.
4 changes: 3 additions & 1 deletion src/app/fdctl/run/tiles/fd_shred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,14 +824,16 @@ unprivileged_init( fd_topo_t * topo,
sign_in->mcache,
sign_in->dcache ) ) );

ulong shred_limit = fd_ulong_if( tile->shred.larger_shred_limits_per_block, 32UL*32UL*1024UL, 32UL*1024UL );
fd_fec_set_t * resolver_sets = fec_sets + (shred_store_mcache_depth+1UL)/2UL + 1UL;
ctx->shredder = NONNULL( fd_shredder_join ( fd_shredder_new ( _shredder, fd_shred_signer, ctx->keyguard_client, (ushort)expected_shred_version ) ) );
ctx->resolver = NONNULL( fd_fec_resolver_join ( fd_fec_resolver_new ( _resolver,
fd_shred_signer, ctx->keyguard_client,
tile->shred.fec_resolver_depth, 1UL,
(shred_store_mcache_depth+3UL)/2UL,
128UL * tile->shred.fec_resolver_depth, resolver_sets,
(ushort)expected_shred_version ) ) );
(ushort)expected_shred_version,
shred_limit ) ) );

ctx->shred34 = shred34;
ctx->fec_sets = fec_sets;
Expand Down
11 changes: 6 additions & 5 deletions src/app/fdctl/run/topos/fd_frankendancer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,12 @@ fd_topo_frankendancer( config_t * config ) {
fd_memcpy( tile->shred.src_mac_addr, config->tiles.net.mac_addr, 6 );
strncpy( tile->shred.identity_key_path, config->consensus.identity_path, sizeof(tile->shred.identity_key_path) );

tile->shred.depth = topo->links[ tile->out_link_id_primary ].depth;
tile->shred.ip_addr = config->tiles.net.ip_addr;
tile->shred.fec_resolver_depth = config->tiles.shred.max_pending_shred_sets;
tile->shred.expected_shred_version = config->consensus.expected_shred_version;
tile->shred.shred_listen_port = config->tiles.shred.shred_listen_port;
tile->shred.depth = topo->links[ tile->out_link_id_primary ].depth;
tile->shred.ip_addr = config->tiles.net.ip_addr;
tile->shred.fec_resolver_depth = config->tiles.shred.max_pending_shred_sets;
tile->shred.expected_shred_version = config->consensus.expected_shred_version;
tile->shred.shred_listen_port = config->tiles.shred.shred_listen_port;
tile->shred.larger_shred_limits_per_block = config->development.bench.larger_shred_limits_per_block;

} else if( FD_UNLIKELY( !strcmp( tile->name, "store" ) ) ) {
tile->store.disable_blockstore = config->development.bench.disable_blockstore;
Expand Down
15 changes: 15 additions & 0 deletions src/ballet/shred/fd_shred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,20 @@ fd_shred_parse( uchar const * const buf,

if( FD_UNLIKELY( sz < header_sz + payload_sz + zero_padding_sz + merkle_proof_sz + previous_merkle_root_sz ) ) return NULL;

/* At this point we know all the fields exist, but we need to sanity
check a few fields that would make a shred illegal. */
if( FD_LIKELY( type & FD_SHRED_TYPEMASK_DATA ) ) {
if( FD_UNLIKELY( (shred->data.flags&0xC0)==0x80 ) ) return NULL;
if( FD_UNLIKELY( (ulong)(shred->data.parent_off)>shred->slot ) ) return NULL;
if( FD_UNLIKELY( (shred->data.parent_off==0) & (shred->slot!=0UL) ) ) return NULL;
if( FD_UNLIKELY( shred->idx<shred->fec_set_idx ) ) return NULL;
} else {
if( FD_UNLIKELY( shred->code.idx>=shred->code.code_cnt ) ) return NULL;
if( FD_UNLIKELY( shred->code.idx> shred->idx ) ) return NULL;
if( FD_UNLIKELY( (shred->code.data_cnt==0)|(shred->code.code_cnt==0) ) ) return NULL;
if( FD_UNLIKELY( shred->code.code_cnt>256 ) ) return NULL;
if( FD_UNLIKELY( (ulong)shred->code.data_cnt+(ulong)shred->code.code_cnt>256 ) ) return NULL; /* I don't see this check in Agave, but it seems necessary */
}

return shred;
}
23 changes: 14 additions & 9 deletions src/ballet/shred/fd_shred.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,16 @@ struct __attribute__((packed)) fd_shred {
/* Hash of the genesis version and historical hard forks of the current chain */
/* 0x4d */ ushort version;

/* Index into the vector of FEC sets for this slot */
/* Index into the vector of FEC sets for this slot. For data shreds, fec_set_idx<=idx. */
/* 0x4f */ uint fec_set_idx;

union {
/* Common data shred header */
struct __attribute__((packed)) {
/* Slot number difference between this block and the parent block.
Always greater than zero. */
parent_off <= slot.
Always greater than zero, except for slot 0, in which case the
previous invariant forces this to be 0. */
/* 0x53 */ ushort parent_off;

/* Bit field (MSB first)
Expand All @@ -214,13 +216,14 @@ struct __attribute__((packed)) fd_shred {

/* Common coding shred header */
struct __attribute__((packed)) {
/* Total number of data shreds in slot */
/* Total number of data shreds in slot. Must be positive. */
/* 0x53 */ ushort data_cnt;

/* Total number of coding shreds in slot */
/* Total number of coding shreds in slot. Must be positive. */
/* 0x55 */ ushort code_cnt;

/* Index within the vector of coding shreds in slot */
/* Index within the vector of coding shreds in slot. In [0,
code_cnt). Also, shred.code.idx <= shred.idx. */
/* 0x57 */ ushort idx;
} code;
};
Expand All @@ -229,11 +232,13 @@ typedef struct fd_shred fd_shred_t;

FD_PROTOTYPES_BEGIN

/* fd_shred_parse: Parses and validates an untrusted shred header.
The provided buffer must be at least FD_SHRED_MIN_SZ bytes long.
/* fd_shred_parse: Parses and validates an untrusted shred stored in
bytes buf[i] for i in [0, sz). sz must be at least FD_SHRED_MIN_SZ
bytes. Allows trailing data.
The returned pointer either equals the input pointer
or is NULL if the given shred is malformed. */
The returned pointer either equals the input pointer or is NULL if
the given shred is malformed or violates any invariants described
above. */
FD_FN_PURE fd_shred_t const *
fd_shred_parse( uchar const * buf,
ulong sz );
Expand Down
2 changes: 2 additions & 0 deletions src/ballet/shred/test_shred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ main( int argc,

/* Test type-specific bounds checks */
if( FD_LIKELY( is_valid )) {
buf[0x53] = buf[0x55] = (uchar)is_code; /* Set data_cnt, code_cnt to 1 */

/* Test merkle node count */
uint merkle_cnt = fd_shred_merkle_cnt( (uchar)i );
FD_TEST( (is_merkle || merkle_cnt==0) && merkle_cnt<=16 );
Expand Down
23 changes: 20 additions & 3 deletions src/disco/shred/fd_fec_resolver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ struct __attribute__((aligned(FD_FEC_RESOLVER_ALIGN))) fd_fec_resolver {
fd_fec_resolver_sign_fn * signer;
void * sign_ctx;

/* max_shred_idx is the exclusive upper bound for shred indices. We
need to reject any shred with an index >= max_shred_idx, but we
also want to reject anything that is part of an FEC set where the
highest index of a shred in the FEC set will be >= max_shred_idx.
*/
ulong max_shred_idx;

/* sha512 and reedsol are used for calculations while adding a shred.
Their state outside a call to add_shred is indeterminate. */
fd_sha512_t sha512[1];
Expand Down Expand Up @@ -187,7 +194,8 @@ fd_fec_resolver_new( void * shmem,
ulong complete_depth,
ulong done_depth,
fd_fec_set_t * sets,
ushort expected_shred_version ) {
ushort expected_shred_version,
ulong max_shred_idx ) {
if( FD_UNLIKELY( (depth==0UL) | (partial_depth==0UL) | (complete_depth==0UL) | (done_depth==0UL) ) ) return NULL;
if( FD_UNLIKELY( (depth>=(1UL<<62)-1UL) | (done_depth>=(1UL<<62)-1UL ) ) ) return NULL;

Expand Down Expand Up @@ -241,6 +249,7 @@ fd_fec_resolver_new( void * shmem,
resolver->expected_shred_version = expected_shred_version;
resolver->signer = signer;
resolver->sign_ctx = sign_ctx;
resolver->max_shred_idx = max_shred_idx;
return shmem;
}

Expand Down Expand Up @@ -352,15 +361,23 @@ int fd_fec_resolver_add_shred( fd_fec_resolver_t * resolver,
}

if( FD_UNLIKELY( shred->version!=resolver->expected_shred_version ) ) return FD_FEC_RESOLVER_SHRED_REJECTED;
if( FD_UNLIKELY( shred_sz<fd_shred_sz( shred ) ) ) return FD_FEC_RESOLVER_SHRED_REJECTED;
if( FD_UNLIKELY( shred->idx>=resolver->max_shred_idx ) ) return FD_FEC_RESOLVER_SHRED_REJECTED;

int is_data_shred = fd_shred_is_data( shred_type );

if( !is_data_shred ) { /* Roughly 50/50 branch */
if( FD_UNLIKELY( (shred->code.data_cnt>FD_REEDSOL_DATA_SHREDS_MAX) | (shred->code.code_cnt>FD_REEDSOL_PARITY_SHREDS_MAX) ) )
return FD_FEC_RESOLVER_SHRED_REJECTED;
if( FD_UNLIKELY( (shred->code.data_cnt==0UL) | (shred->code.code_cnt==0UL) ) ) return FD_FEC_RESOLVER_SHRED_REJECTED;
if( FD_UNLIKELY( (shred->code.data_cnt==0UL) | (shred->code.code_cnt==0UL) ) )
return FD_FEC_RESOLVER_SHRED_REJECTED;
if( FD_UNLIKELY( (ulong)shred->fec_set_idx+(ulong)shred->code.data_cnt>=resolver->max_shred_idx ) )
return FD_FEC_RESOLVER_SHRED_REJECTED;
if( FD_UNLIKELY( (ulong)shred->idx + (ulong)shred->code.code_cnt - (ulong)shred->code.idx>=resolver->max_shred_idx ) )
return FD_FEC_RESOLVER_SHRED_REJECTED;
}


/* For the purposes of the shred header, tree_depth means the number
of nodes, counting the leaf but excluding the root. For bmtree,
depth means the number of layers, which counts both. */
Expand Down Expand Up @@ -496,7 +513,7 @@ int fd_fec_resolver_add_shred( fd_fec_resolver_t * resolver,

/* Copy the shred to memory the FEC resolver owns */
uchar * dst = fd_ptr_if( is_data_shred, ctx->set->data_shreds[ in_type_idx ], ctx->set->parity_shreds[ in_type_idx ] );
fd_memcpy( dst, shred, shred_sz );
fd_memcpy( dst, shred, fd_shred_sz( shred ) );

/* If the shred needs a retransmitter signature, set it */
if( FD_UNLIKELY( fd_shred_is_resigned( shred_type ) ) ) {
Expand Down
11 changes: 8 additions & 3 deletions src/disco/shred/fd_fec_resolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,12 @@ ulong fd_fec_resolver_align ( void );
output parameters of _add_shred. The FEC resolver will reject any
shreds with a shred version that does not match the value provided
for expected_shred_version. Shred versions are always non-zero, so
expected_shred_version must be non-zero. Returns shmem on success
and NULL on failure (logs details). */
expected_shred_version must be non-zero. The FEC resolver will also
reject any shred that seems to be part of a block containing more
than max_shred_idx data or parity shreds. Since shred_idx is a uint,
it doesn't really make sense to have max_shred_idx > UINT_MAX, and
max_shred_idx==0 rejects all shreds. Returns shmem on success and
NULL on failure (logs details). */
void *
fd_fec_resolver_new( void * shmem,
fd_fec_resolver_sign_fn * signer,
Expand All @@ -134,7 +138,8 @@ fd_fec_resolver_new( void * shmem,
ulong complete_depth,
ulong done_depth,
fd_fec_set_t * sets,
ushort expected_shred_version );
ushort expected_shred_version,
ulong max_shred_idx );

fd_fec_resolver_t * fd_fec_resolver_join( void * shmem );

Expand Down
Loading

0 comments on commit 9d12a76

Please sign in to comment.