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

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

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.

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

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
participants (4)
-
Jiri Denemark
-
Pavel Hrdina
-
Peter Krempa
-
Simon Kobyda