Re: [PATCH] udf: validate VAT header length against the VAT inode size

[Jan Kara]

On Fri 12-06-26 02:53:31, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> 
> udf_load_vat() takes the virtual partition's start offset straight from
> the on-disk VAT 2.0 header without checking it against the VAT inode
> size:
> 
> 	map->s_type_specific.s_virtual.s_start_offset =
> 		le16_to_cpu(vat20->lengthHeader);
> 	map->s_type_specific.s_virtual.s_num_entries =
> 		(sbi->s_vat_inode->i_size -
> 			map->s_type_specific.s_virtual.s_start_offset) >> 2;
> 
> lengthHeader is a fully attacker-controlled 16-bit value.  If it exceeds
> the VAT inode size, the s_num_entries subtraction underflows to a huge
> count, which defeats the "block > s_num_entries" bound in
> udf_get_pblock_virt15(); and on the ICB-inline path that function reads
> 
> 	((__le32 *)(iinfo->i_data + s_start_offset))[block]
> 
> so a large s_start_offset indexes past the inode's in-ICB data.  Mounting
> a crafted UDF image with a virtual (VAT) partition then triggers an
> out-of-bounds read.
> 
> Reject a VAT whose header length does not leave room for at least one
> entry within the VAT inode.
> 
> Fixes: fa5e08156335 ("udf: Handle VAT packed inside inode properly")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>

Thanks. I've added the patch to my tree. I'll just note that we need a
similar check for older format of VAT handled in that function as well.
I'll add that.

								Honza

> ---
>  fs/udf/super.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index cad2e15d633b..561eace476ab 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -1263,6 +1263,14 @@ static int udf_load_vat(struct super_block *sb, int p_index, int type1_index)
>  
>  		map->s_type_specific.s_virtual.s_start_offset =
>  			le16_to_cpu(vat20->lengthHeader);
> +		if (map->s_type_specific.s_virtual.s_start_offset +
> +		    sizeof(__le32) > sbi->s_vat_inode->i_size) {
> +			udf_err(sb, "Corrupted VAT header length %u (VAT inode size %lld)\n",
> +				map->s_type_specific.s_virtual.s_start_offset,
> +				sbi->s_vat_inode->i_size);
> +			brelse(bh);
> +			return -EFSCORRUPTED;
> +		}
>  		map->s_type_specific.s_virtual.s_num_entries =
>  			(sbi->s_vat_inode->i_size -
>  				map->s_type_specific.s_virtual.
> 
> ---
> base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
> change-id: 20260612-b4-disp-9a2317ee-0042af154672
> 
> Best regards,
> -- 
> Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> 
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR