Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/qstring.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qobject/qstring.c b/qobject/qstring.c
index 18b8eb82f8..7990569c5a 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -41,17 +41,19 @@ QString *qstring_from_substr(const char *str, size_t start, size_t end)
 {
     QString *qstring;
 
+    assert(start <= end + 1);
+
     qstring = g_malloc(sizeof(*qstring));
     qobject_init(QOBJECT(qstring), QTYPE_QSTRING);
 
     qstring->length = end - start + 1;
     qstring->capacity = qstring->length;
 
+    assert(qstring->capacity < SIZE_MAX);
     qstring->string = g_malloc(qstring->capacity + 1);
     memcpy(qstring->string, str + start, qstring->length);
     qstring->string[qstring->length] = 0;
 
-
     return qstring;
 }
 
@@ -68,7 +70,9 @@ QString *qstring_from_str(const char *str)
 static void capacity_increase(QString *qstring, size_t len)
 {
     if (qstring->capacity < (qstring->length + len)) {
+        assert(len <= SIZE_MAX - qstring->capacity);
         qstring->capacity += len;
+        assert(qstring->capacity + len <= SIZE_MAX / 2);
         qstring->capacity *= 2; /* use exponential growth */
 
         qstring->string = g_realloc(qstring->string, qstring->capacity + 1);
-- 
2.17.1

On 07/26/2018 01:18 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   qobject/qstring.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/qobject/qstring.c b/qobject/qstring.c
> index 18b8eb82f8..7990569c5a 100644
> --- a/qobject/qstring.c
> +++ b/qobject/qstring.c
> @@ -41,17 +41,19 @@ QString *qstring_from_substr(const char *str, size_t start, size_t end)
>   {
>       QString *qstring;
>   
> +    assert(start <= end + 1);

end + 1 can overflow size_t, but it is unsigned so well-defined, and the 
assert will trigger as desired.

> @@ -68,7 +70,9 @@ QString *qstring_from_str(const char *str)
>   static void capacity_increase(QString *qstring, size_t len)
>   {
>       if (qstring->capacity < (qstring->length + len)) {
> +        assert(len <= SIZE_MAX - qstring->capacity);
>           qstring->capacity += len;

You've asserted that this addition won't overflow...

> +        assert(qstring->capacity + len <= SIZE_MAX / 2);

...but now that qstring->capacity is larger, this could overflow. Do you 
really need the +len in here, given that...

>           qstring->capacity *= 2; /* use exponential growth */

...you are really only trying to prevent overflow of doubling 
qstring->capacity without adding yet another len in the mix?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Eric Blake <eblake@redhat.com> writes:

> On 07/26/2018 01:18 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   qobject/qstring.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/qobject/qstring.c b/qobject/qstring.c
>> index 18b8eb82f8..7990569c5a 100644
>> --- a/qobject/qstring.c
>> +++ b/qobject/qstring.c
>> @@ -41,17 +41,19 @@ QString *qstring_from_substr(const char *str, size_t start, size_t end)
>>   {
>>       QString *qstring;
>>   +    assert(start <= end + 1);
>
> end + 1 can overflow size_t, but it is unsigned so well-defined, and
> the assert will trigger as desired.
>
>> @@ -68,7 +70,9 @@ QString *qstring_from_str(const char *str)
>>   static void capacity_increase(QString *qstring, size_t len)
>>   {
>>       if (qstring->capacity < (qstring->length + len)) {
>> +        assert(len <= SIZE_MAX - qstring->capacity);
>>           qstring->capacity += len;
>
> You've asserted that this addition won't overflow...
>
>> +        assert(qstring->capacity + len <= SIZE_MAX / 2);
>
> ...but now that qstring->capacity is larger, this could overflow. Do
> you really need the +len in here, given that...
>
>>           qstring->capacity *= 2; /* use exponential growth */
>
> ...you are really only trying to prevent overflow of doubling
> qstring->capacity without adding yet another len in the mix?

You're right, my assertion is broken.  v2 coming.