diff mbox series

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

Message ID 20241011031937.92216-2-demeng@redhat.com (mailing list archive)
State New
Headers show
Series qemu-ga: Fix some potential issues find by coverity | expand

Commit Message

Dehan Meng Oct. 11, 2024, 3:19 a.m. UTC 
Signed-off-by: Dehan Meng <demeng@redhat.com>
---
 qga/commands-linux.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé Oct. 14, 2024, 9:55 a.m. UTC | #1 
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
diff mbox series

Patch

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);