Re: [Qemu-devel] [PATCH 7/7] smbios: Check R in -smbios type=0, release=R parses okay

[Laszlo Ersek <lersek@redhat.com>]

On 06/06/13 18:27, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/i386/smbios.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
> index 68bd6d0..88a1360 100644
> --- a/hw/i386/smbios.c
> +++ b/hw/i386/smbios.c
> @@ -140,7 +140,10 @@ static void smbios_build_type_0_fields(const char *t)
>                                       bios_release_date_str),
>                           buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "release", t)) {
> -        sscanf(buf, "%hhd.%hhd", &major, &minor);
> +        if (sscanf(buf, "%hhd.%hhd", &major, &minor) != 2) {
> +            error_report("Invalid release");
> +            exit(1);
> +        }
>          smbios_add_field(0, offsetof(struct smbios_type_0,
>                                       system_bios_major_release),
>                           &major, 1);
> 

Right. OTOH if any of the decimal strings provided doesn't fit into the
space provided (eg. you pass "256" for an "unsigned char" which happens
to be uint8_t), the behavior is undefined anyway. sscanf() cannot be
used with "untrusted" data. ("... if the result of the conversion cannot
be represented in the space provided, the behavior is undefined.")

Reviewed-by: Laszlo Ersek <lersek@redhat.com>