Re: [Qemu-devel] [PATCH v1] multiboot: validate multiboot header address values

[Paolo Bonzini]

On 07/09/2017 08:32, P J P wrote:
> From: Prasad J Pandit <address@hidden>
> 
> While loading kernel via multiboot-v1 image, (flags & 0x00010000)
> indicates that multiboot header contains valid addresses to load
> the kernel image. These addresses are used to compute kernel
> size and kernel text offset in the OS image. Validate these
> address values to avoid an OOB access issue.
> 
> This is CVE-2017-14167.
> 
> Reported-by: Thomas Garnier <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  hw/i386/multiboot.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> Update: add CVE-ID to the commit message.
> 
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index 6001f4caa2..c7b70c91d5 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -221,15 +221,34 @@ int load_multiboot(FWCfgState *fw_cfg,
>          uint32_t mh_header_addr = ldl_p(header+i+12);
>          uint32_t mh_load_end_addr = ldl_p(header+i+20);
>          uint32_t mh_bss_end_addr = ldl_p(header+i+24);
> +
>          mh_load_addr = ldl_p(header+i+16);
> +        if (mh_header_addr < mh_load_addr) {
> +            fprintf(stderr, "invalid mh_load_addr address\n");
> +            exit(1);
> +        }
> +
>          uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
>          uint32_t mb_load_size = 0;
>          mh_entry_addr = ldl_p(header+i+28);
>  
>          if (mh_load_end_addr) {
> +            if (mh_bss_end_addr < mh_load_addr) {
> +                fprintf(stderr, "invalid mh_bss_end_addr address\n");
> +                exit(1);
> +            }
>              mb_kernel_size = mh_bss_end_addr - mh_load_addr;
> +
> +            if (mh_load_end_addr < mh_load_addr) {
> +                fprintf(stderr, "invalid mh_load_end_addr address\n");
> +                exit(1);
> +            }
>              mb_load_size = mh_load_end_addr - mh_load_addr;
>          } else {
> +            if (kernel_file_size < mb_kernel_text_offset) {
> +                fprintf(stderr, "invalid kernel_file_size\n");
> +                exit(1);
> +            }
>              mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
>              mb_load_size = mb_kernel_size;
>          }
> 

Queued, thanks.

Paolo