[Qemu-devel] [PATCHv4] qdev: Validate hex properties

[Qemu-devel] [PATCHv4] qdev: Validate hex properties

[Hannes Reinecke]

strtoul(l) might overflow, in which case it'll return '-1' and set
the appropriate error code. So update the calls to strtoul(l) when
parsing hex properties to avoid silent overflows.
And we should be using an intermediate variable to avoid clobbering
of the passed-in point on error.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 hw/core/qdev-properties.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dc8ae69..6e4e9f3 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -191,6 +191,7 @@ PropertyInfo qdev_prop_uint8 = {
 
 static int parse_hex8(DeviceState *dev, Property *prop, const char *str)
 {
+    unsigned long val;
     uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
@@ -198,11 +199,18 @@ static int parse_hex8(DeviceState *dev, Property *prop, const char *str)
         return -EINVAL;
     }
 
-    *ptr = strtoul(str, &end, 16);
+    errno = 0;
+    val = strtoul(str, &end, 16);
+    if (errno) {
+        return -errno;
+    }
+    if (val > UINT8_MAX) {
+        return -ERANGE;
+    }
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
     }
-
+    *ptr = val;
     return 0;
 }
 
@@ -322,6 +330,7 @@ PropertyInfo qdev_prop_int32 = {
 
 static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
 {
+    unsigned long val;
     uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
@@ -329,11 +338,18 @@ static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
         return -EINVAL;
     }
 
-    *ptr = strtoul(str, &end, 16);
+    errno = 0;
+    val = strtoul(str, &end, 16);
+    if (errno) {
+        return -errno;
+    }
+    if (val > UINT32_MAX) {
+        return -ERANGE;
+    }
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
     }
-
+    *ptr = val;
     return 0;
 }
 
@@ -389,6 +405,7 @@ PropertyInfo qdev_prop_uint64 = {
 
 static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
 {
+    unsigned long long val;
     uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
@@ -396,11 +413,18 @@ static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
         return -EINVAL;
     }
 
-    *ptr = strtoull(str, &end, 16);
+    errno = 0;
+    val = strtoull(str, &end, 16);
+    if (errno) {
+        return -errno;
+    }
+    if (val > UINT64_MAX) {
+        return -ERANGE;
+    }
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
     }
-
+    *ptr = val;
     return 0;
 }
 
-- 
1.8.1.4

Re: [Qemu-devel] [PATCHv4] qdev: Validate hex properties

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On 11/28/2013 11:48 PM, Hannes Reinecke wrote:
> strtoul(l) might overflow, in which case it'll return '-1' and set
> the appropriate error code. So update the calls to strtoul(l) when
> parsing hex properties to avoid silent overflows.
> And we should be using an intermediate variable to avoid clobbering
> of the passed-in point on error.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  hw/core/qdev-properties.c | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]