Re: [PATCH 3/8] vpn: Avoid using uninitialized prefix_len

Hi Denis!

On 4/14/25 18:37, Denis Kenzior wrote:
> Hi Jussi,
> 
> On 4/10/25 10:58 AM, Jussi Laakkonen wrote:
>> Clang complains about the prefix_len being possibly uninitialized. Set
>> the max for IPv6 prefix len as the value.
>> ---
>>   plugins/vpn.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/plugins/vpn.c b/plugins/vpn.c
>> index 42396d2a..9664c82f 100644
>> --- a/plugins/vpn.c
>> +++ b/plugins/vpn.c
>> @@ -4,6 +4,7 @@
>>    *
>>    *  Copyright (C) 2012-2013  Intel Corporation. All rights reserved.
>>    *  Copyright (C) 2019-2021  Jolla Ltd. All rights reserved.
>> + *  Copyright (C) 2025  Jolla Mobile Ltd
>>    *
>>    *  This program is free software; you can redistribute it and/or 
>> modify
>>    *  it under the terms of the GNU General Public License version 2 as
>> @@ -388,7 +389,7 @@ static int extract_ip(DBusMessageIter *array, int 
>> family,
>>   {
>>       DBusMessageIter dict;
>>       char *address = NULL, *gateway = NULL, *netmask = NULL, *peer = 
>> NULL;
>> -    unsigned char prefix_len;
>> +    unsigned char prefix_len = 128;
> 
> How does this help?  You may want to share the actual error verbatim. 
> 'prefix_len' is set from 'atoi(netmask)'.
> 
> The use of atoi seems to be pretty dangerous here though.


Ah, now I remember, the complaint was because of this:
https://github.com/sailfishos/connman/commit/2fe641f0f7d6deb7b64248c134312db9e564f366

Which I forgot to push. And yes, atoi is dangerous here and the value 
should be read and set from the D-Bus message content directly.

I'll send this patch too asap. Thanks for the comments, it slipped my 
mind completely.

> 
>>
>>       if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
>>           return -EINVAL;
>> -- 
>> 2.39.5
>>
>>
> 

Cheers,
  Jussi