virXPathString() can return NULL so we need to check it before calling strcmp()

Found by Linux Verification Center (linuxtesting.org) with Svace.

Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 tools/virsh-completer-domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 0a3a113dca..abacfd04c1 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -891,7 +891,7 @@ virshDomainConsoleCompleter(vshControl *ctl,
             ctxt->node = parallels[i - nserials];
 
         type = virXPathString("string(./@type)", ctxt);
-        if (STRNEQ(type, "pty"))
+        if (!type || STRNEQ(type, "pty"))
             continue;
 
         tmp[offset++] = virXPathString("string(./alias/@name)", ctxt);
-- 
2.42.4

On Mon, Apr 14, 2025 at 14:56:03 +0300, Alexander Kuznetsov wrote:
> virXPathString() can return NULL so we need to check it before calling strcmp()
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  tools/virsh-completer-domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
> index 0a3a113dca..abacfd04c1 100644
> --- a/tools/virsh-completer-domain.c
> +++ b/tools/virsh-completer-domain.c
> @@ -891,7 +891,7 @@ virshDomainConsoleCompleter(vshControl *ctl,
>              ctxt->node = parallels[i - nserials];
>  
>          type = virXPathString("string(./@type)", ctxt);
> -        if (STRNEQ(type, "pty"))
> +        if (!type || STRNEQ(type, "pty"))

Alternative is to use STRNEQ_NULLABLE.

>              continue;
>  
>          tmp[offset++] = virXPathString("string(./alias/@name)", ctxt);
> -- 
> 2.42.4
>

v2:
- switch to use STRNEQ_NULLABLE instead of simple nullness check

Alexander Kuznetsov (1):
  virsh: prevent potential NULL dereference

 tools/virsh-completer-domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.42.4

virXPathString() can return NULL so we need to use STRNEQ_NULLABLE here

Found by Linux Verification Center (linuxtesting.org) with Svace.

Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 tools/virsh-completer-domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 0a3a113dca..b6a4d85ee8 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -891,7 +891,7 @@ virshDomainConsoleCompleter(vshControl *ctl,
             ctxt->node = parallels[i - nserials];
 
         type = virXPathString("string(./@type)", ctxt);
-        if (STRNEQ(type, "pty"))
+        if (STRNEQ_NULLABLE(type, "pty"))
             continue;
 
         tmp[offset++] = virXPathString("string(./alias/@name)", ctxt);
-- 
2.42.4

On 4/14/25 15:32, Alexander Kuznetsov wrote:
> virXPathString() can return NULL so we need to use STRNEQ_NULLABLE here
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Dmitry Fedin <d.fedin@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  tools/virsh-completer-domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and merged.

Michal