On 02/02/2017 08:14 AM, Martin Kletzander wrote:
With that users could access files outside /dev/shm. That itself isn't a security problem, but might cause some errors we want to avoid. So let's forbid slashes as we do with domain and volume names and also mention that in the schema.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1395496
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/schemas/domaincommon.rng | 6 +++++- src/qemu/qemu_process.c | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-)
This was really familiar... hmm.. oh yeah... Can/should virXMLCheckIllegalChars be used? See commits ae381879f, dc40dd60, and e1b81968 Likewise, makes me wonder if the *.rng for all those would need some sort of updating to remove chance that a '\n' exists like you've done here for the '/' character. Secondary of course is should the failure be in Parse rather than checking at startup time? I agree in principal with what's be done, just the "where" it should be done. John
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cc6e0d0c0d65..00cdc93bca59 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3598,7 +3598,11 @@
<define name="shmem"> <element name="shmem"> - <attribute name="name"/> + <attribute name="name"> + <data type="string"> + <param name="pattern">[^/]*</param> + </data> + </attribute> <interleave> <optional> <element name="model"> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 184440dc1af6..0f63668100a6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4586,6 +4586,26 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm,
static int +qemuProcessStartValidateShmem(virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->nshmems; i++) { + virDomainShmemDefPtr shmem = vm->def->shmems[i]; + + if (strchr(shmem->name, '/')) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("shmem name '%s' must not contain '/'"), + shmem->name); + return -1; + } + } + + return 0; +} + + +static int qemuProcessStartValidateXML(virQEMUDriverPtr driver, virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, @@ -4661,6 +4681,9 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, if (qemuProcessStartValidateVideo(vm, qemuCaps) < 0) return -1;
+ if (qemuProcessStartValidateShmem(vm) < 0) + return -1; + VIR_DEBUG("Checking for any possible (non-fatal) issues");
qemuProcessStartWarnShmem(vm);