Re: [Qemu-devel] [PATCH v2] vmdk: Fix "%x" to PRIx32 in format strings for cid

[Eric Blake]

On 04/17/2014 04:43 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <address@hidden>
> 
> ---
> v2: PRIx32 -> SCNx32. (Kevin)
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---

> +++ b/block/vmdk.c
> @@ -262,7 +262,7 @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int 
> parent)
>      p_name = strstr(desc, cid_str);
>      if (p_name != NULL) {
>          p_name += cid_str_size;
> -        sscanf(p_name, "%x", &cid);
> +        sscanf(p_name, "%" SCNx32, &cid);

sscanf() has undefined behavior on integer overflow.  This is not the
only vulnerable site in the code base, but if you are ever reading from
external input, and the ascii string being parsed does not fit in the
variable requested by SCNx32, you risk silently parsing the wrong
number.  It is always safer to use the strtol family (or a sane wrapper
thereof that gets errno handling correct) for parsing strings into
integers.  That said, I'm not going to reject this patch for using
sscanf, so much as suggest that you look into a followup patch to avoid it.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature