Re: [PATCH] udf: validate sparing table length as an entry count, not a byte count

[Jan Kara]

On Fri 12-06-26 01:40:01, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> 
> udf_load_sparable_map() accepts a sparing table when
> 
> 	sizeof(*st) + le16_to_cpu(st->reallocationTableLen) > sb->s_blocksize
> 
> is false, i.e. it treats reallocationTableLen as a number of BYTES that
> must fit in the block.  But the table is walked as an array of 8-byte
> sparingEntry elements:
> 
> 	for (i = 0; i < le16_to_cpu(st->reallocationTableLen); i++) {
> 		struct sparingEntry *entry = &st->mapEntry[i];
> 		... entry->origLocation ...
> 	}
> 
> in udf_get_pblock_spar15() and udf_relocate_blocks().  A
> reallocationTableLen of N therefore passes the check whenever
> sizeof(*st) + N <= blocksize, yet the consumers index
> sizeof(*st) + N * sizeof(struct sparingEntry) bytes -- up to ~8x the
> block.  On a crafted UDF image this is an out-of-bounds read in
> udf_get_pblock_spar15(); udf_relocate_blocks() additionally feeds the
> same length to udf_update_tag(), whose crc_itu_t() reads far past the
> block, and its memmove() through st->mapEntry[] is an out-of-bounds
> write.
> 
> Validate reallocationTableLen as the entry count it is, with
> struct_size().
> 
> Fixes: 1df2ae31c724 ("udf: Fortify loading of sparing table")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>

Good catch. Thanks for the fix. I've added the patch to my tree.

								Honza

> ---
>  fs/udf/super.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index cad2e15d633b..d75bfc733975 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -1418,7 +1418,8 @@ static int udf_load_sparable_map(struct super_block *sb,
>  		if (ident != 0 ||
>  		    strncmp(st->sparingIdent.ident, UDF_ID_SPARING,
>  			    strlen(UDF_ID_SPARING)) ||
> -		    sizeof(*st) + le16_to_cpu(st->reallocationTableLen) >
> +		    struct_size(st, mapEntry,
> +				le16_to_cpu(st->reallocationTableLen)) >
>  							sb->s_blocksize) {
>  			brelse(bh);
>  			continue;
> 
> ---
> base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
> change-id: 20260612-b4-disp-91780c4e-fb52a746d9df
> 
> Best regards,
> -- 
> Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> 
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR