[v1] conf: virDomainDefValidateInternal prohibit some characters in shmem name

[libvirt] [PATCH] conf: virDomainDefValidateInternal prohibit some characters in shmem name

Posted by Simon Kobyda 5 years, 9 months ago

XML shmem name will not include characters '/', '.' or '..', as shmem name
is used in a path
https://bugzilla.redhat.com/show_bug.cgi?id=1192400
---
 src/conf/domain_conf.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7ab2953d83..3f580525bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6107,6 +6107,8 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def)
 static int
 virDomainDefValidateInternal(const virDomainDef *def)
 {
+    size_t i;
+
     if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
         return -1;
 
@@ -6136,6 +6138,17 @@ virDomainDefValidateInternal(const virDomainDef *def)
         return -1;
     }
 
+    for (i = 0; i < def->nshmems; i++) {
+        if (strchr(def->shmems[i]->name, '/'))
+            return -1;
+
+        if (strchr(def->shmems[i]->name, '.'))
+            return -1;
+
+        if (strstr(def->shmems[i]->name, ".."))
+            return -1;
+    }
+
     if (virDomainDefLifecycleActionValidate(def) < 0)
         return -1;
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] conf: virDomainDefValidateInternal prohibit some characters in shmem name

Posted by Peter Krempa 5 years, 9 months ago

On Tue, Jul 10, 2018 at 13:37:34 +0200, Simon Kobyda wrote:
> XML shmem name will not include characters '/', '.' or '..', as shmem name
> is used in a path
> https://bugzilla.redhat.com/show_bug.cgi?id=1192400
> ---
>  src/conf/domain_conf.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7ab2953d83..3f580525bb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6107,6 +6107,8 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def)
>  static int
>  virDomainDefValidateInternal(const virDomainDef *def)
>  {
> +    size_t i;
> +
>      if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
>          return -1;
>  
> @@ -6136,6 +6138,17 @@ virDomainDefValidateInternal(const virDomainDef *def)
>          return -1;
>      }
>  
> +    for (i = 0; i < def->nshmems; i++) {
> +        if (strchr(def->shmems[i]->name, '/'))
> +            return -1;
> +
> +        if (strchr(def->shmems[i]->name, '.'))
> +            return -1;
> +
> +        if (strstr(def->shmems[i]->name, ".."))
> +            return -1;

This is dead code since a single dot is rejected by the above
statement.

Also you need to report an error, since the user would not know what is
wrong here.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] conf: virDomainDefValidateInternal prohibit some characters in shmem name

Posted by Pavel Hrdina 5 years, 9 months ago

On Tue, Jul 10, 2018 at 01:45:33PM +0200, Peter Krempa wrote:
> On Tue, Jul 10, 2018 at 13:37:34 +0200, Simon Kobyda wrote:
> > XML shmem name will not include characters '/', '.' or '..', as shmem name
> > is used in a path
> > https://bugzilla.redhat.com/show_bug.cgi?id=1192400
> > ---
> >  src/conf/domain_conf.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 7ab2953d83..3f580525bb 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -6107,6 +6107,8 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def)
> >  static int
> >  virDomainDefValidateInternal(const virDomainDef *def)
> >  {
> > +    size_t i;
> > +
> >      if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
> >          return -1;
> >  
> > @@ -6136,6 +6138,17 @@ virDomainDefValidateInternal(const virDomainDef *def)
> >          return -1;
> >      }
> >  
> > +    for (i = 0; i < def->nshmems; i++) {
> > +        if (strchr(def->shmems[i]->name, '/'))
> > +            return -1;
> > +
> > +        if (strchr(def->shmems[i]->name, '.'))
> > +            return -1;
> > +
> > +        if (strstr(def->shmems[i]->name, ".."))
> > +            return -1;
> 
> This is dead code since a single dot is rejected by the above
> statement.
> 
> Also you need to report an error, since the user would not know what is
> wrong here.

Instead of strchr you can use virStringHasChars(), so in this case:

virStringHasChars(def->shmems[i]->name, "/.")

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] conf: virDomainDefValidateInternal prohibit some characters in shmem name

Posted by Jiri Denemark 5 years, 9 months ago

On Tue, Jul 10, 2018 at 13:37:34 +0200, Simon Kobyda wrote:
> XML shmem name will not include characters '/', '.' or '..', as shmem name
> is used in a path
> https://bugzilla.redhat.com/show_bug.cgi?id=1192400
> ---
>  src/conf/domain_conf.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7ab2953d83..3f580525bb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6107,6 +6107,8 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def)
>  static int
>  virDomainDefValidateInternal(const virDomainDef *def)
>  {
> +    size_t i;
> +
>      if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
>          return -1;
>  
> @@ -6136,6 +6138,17 @@ virDomainDefValidateInternal(const virDomainDef *def)
>          return -1;
>      }
>  
> +    for (i = 0; i < def->nshmems; i++) {
> +        if (strchr(def->shmems[i]->name, '/'))
> +            return -1;
> +
> +        if (strchr(def->shmems[i]->name, '.'))
> +            return -1;
> +
> +        if (strstr(def->shmems[i]->name, ".."))
> +            return -1;
> +    }
> +

Why do you want to forbid "." and ".." in the name? Sure '/' should not
be anywhere in the name, but what's wrong with dots? The name as a whole
should not be "." or ".." (as correctly stated in the bugzilla), but
there's nothing wrong with, e.g., "some.name.with.dots...", is it?

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list