CVE-2015-5313: don't allow '/' in filesystem volume names

[libvirt] [PATCH] CVE-2015-5313: storage: don’t allow ‘/’ in filesystem volume names****Eric Blake eblake at redhat.com
Fri Dec 11 23:38:30 UTC 2015

• Previous message (by thread): [libvirt] [PATCH 0/5] virStorageBackendWipeLocal cleanups
• Next message (by thread): [libvirt] [PATCH] virNetDevMacVLanTapSetup: Work around older systems
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 at 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 © 2007-2014 Red Hat, Inc.

• * Copyright © 2007-2015 Red Hat, Inc. * Copyright © 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

• Previous message (by thread): [libvirt] [PATCH 0/5] virStorageBackendWipeLocal cleanups
• Next message (by thread): [libvirt] [PATCH] virNetDevMacVLanTapSetup: Work around older systems
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the libvir-list mailing list