Re: [PATCH v2 1/4] sscanf return values are checked to ensure correct parsing.

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Oct 11, 2024 at 11:19:34AM +0800, Dehan Meng wrote:
> Signed-off-by: Dehan Meng <demeng@redhat.com>
> ---
>  qga/commands-linux.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> index 51d5e3d927..2c2b5f4ff2 100644
> --- a/qga/commands-linux.c
> +++ b/qga/commands-linux.c
> @@ -2103,7 +2103,9 @@ static char *hexToIPAddress(const void *hexValue, int is_ipv6)
>          int i;
>  
>          for (i = 0; i < 16; i++) {
> -            sscanf(&hexStr[i * 2], "%02hhx", &in6.s6_addr[i]);
> +            if (sscanf(&hex_str[i * 2], "%02hhx", &in6.s6_addr[i]) != 1) {
> +                return NULL;
> +            }
>          }
>          inet_ntop(AF_INET6, &in6, addr, INET6_ADDRSTRLEN);

None of the callers of this function are expecting it to return NULL.

eg this code:

                networkroute->destination = hexToIPAddress(Destination, 1);
                networkroute->metric = Metric;
                networkroute->source = hexToIPAddress(Source, 1);
                networkroute->desprefixlen = g_strdup_printf(
                    "%d", DesPrefixlen
                );
                networkroute->srcprefixlen = g_strdup_printf(
                    "%d", SrcPrefixlen
                );
                networkroute->nexthop = hexToIPAddress(NextHop, 1);

The QAPI schema allows 'source' and 'nexthop' to be optional so those
two are fnie.

The 'destination' field is marked as mandatory thoug, so must not
be NULL.

IOW, in the calls we need to check for NULL, and skip adding the
entire route object if 'destniation' is NULL.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|