Re: [PATCH] hw/i386: Improve bounds checking in OVMF table parsing

[Philippe Mathieu-Daudé]

On 14/2/22 13:08, Dov Murik wrote:
> When pc_system_parse_ovmf_flash() parses the optional GUIDed table in
> the end of the OVMF flash memory area, the table length field is checked
> for sizes that are too small, but doesn't error on sizes that are too
> big (bigger than the flash content itself).
>
> Add a check for maximal size of the OVMF table, and add an error report
> in case the size is invalid.
>
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> ---
>   hw/i386/pc_sysfw_ovmf.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c
> index f4dd92c588..0663f3f54a 100644
> --- a/hw/i386/pc_sysfw_ovmf.c
> +++ b/hw/i386/pc_sysfw_ovmf.c
> @@ -24,6 +24,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>   #include "hw/i386/pc.h"
>   #include "cpu.h"
>   
> @@ -66,7 +67,13 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
>       ptr -= sizeof(uint16_t);
>       tot_len = le16_to_cpu(*(uint16_t *)ptr) - sizeof(guid) - sizeof(uint16_t);
>   
> -    if (tot_len <= 0) {
> +    if (tot_len < 0 || tot_len > flash_size - 50) {

Please use a definition instead of this magic '50' number.

> +        error_report("OVMF table has invalid size %d", tot_len);
> +        return;
> +    }
> +
> +    if (tot_len == 0) {
> +        /* no entries in the OVMF table */
>           return;
>       }
>   
>
> base-commit: 48033ad678ae2def43bf0d543a2c4c3d2a93feaf