The libvirt file system storage driver determines what file to
act on by concatenating the pool location with the volume name.
If a user is able to pick names like "../../../etc/passwd", then
they can escape the bounds of the pool. For that matter,
virStoragePoolListVolumes() doesn't descend into subdirectories,
so a user really shouldn't use a name with a slash.
Normally, only privileged users can coerce libvirt into creating
or opening existing files using the virStorageVol APIs; and such
users already have full privilege to create any domain XML (so it
is not an escalation of privilege). But in the case of
fine-grained ACLs, it is feasible that a user can be granted
storage_vol:create but not domain:write, and it violates
assumptions if such a user can abuse libvirt to access files
outside of the storage pool.
Therefore, prevent all use of volume names that contain "/",
whether or not such a name is actually attempting to escape the
pool.
This changes things from:
$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
Vol ../../../../../../etc/haha created
$ rm /etc/haha
to:
$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128
error: Failed to create vol ../../../../../../etc/haha
error: Requested operation is not valid: volume name '../../../../../../etc/haha'
cannot contain '/'
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
This has been reviewed on the libvirt security list, where it
was assigned a CVE. Fortunately, this could only be used for
an escalation of privileges under fine-grained ACLs (which is
not an out-of-the-box config).
I will go ahead and push this to master as well as all the
active maint branches back to the introduction of ACLs.
src/storage/storage_backend_fs.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index c71c724..bb3b62a 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1,7 +1,7 @@
/*
* storage_backend_fs.c: storage backend for FS and directory handling
*
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2015 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -1057,6 +1057,14 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
else
vol->type = VIR_STORAGE_VOL_FILE;
+ /* Volumes within a directory pools are not recursive; do not
+ * allow escape to ../ or a subdir */
+ if (strchr(vol->name, '/')) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("volume name '%s' cannot contain
'/'"), vol->name);
+ return -1;
+ }
+
VIR_FREE(vol->target.path);
if (virAsprintf(&vol->target.path, "%s/%s",
pool->def->target.path,
--
2.4.3
Show replies by date