Un-sanitized eval statement in EMC volume driver

[Travis McPeak]

In the EMC volume driver, the function _find_lun calls eval on a string without doing any kind of input sanitization:

https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/emc/emc_vmax_common.py#L1277

I haven't tracked down all the ways that "provider_location" can be set, but if an attacker is able to be set to something like "__import__('os').system('rm -rf /etc')" the etc directory will be removed with the privileges of the user that Cinder runs as.

Maybe somebody more familiar with Cinder and what "provider_location" does can chime in to whether it can be set to something malicious or not, and if so, how difficult that is.

Eval should never be called on un-sanitized or untrusted input.

If we are sure that there is no way for a malicious user to set this parameter, it can be made public, and should be viewed as a security hardening improvement.

[Thierry Carrez (ttx)]

Was submitted as public bug on the duplicate.

[Xing Yang (xing-yang)]

provider_location is set by the driver. In the VMAX driver, this is the object path of a volume in SMI-S term. It is in a very specific format. It will be set when the volume is created. When retrieving an existing volume using this object path stored in provider_location, the volume can be found faster on the array using the SMI-S query. If provider_location is changed by someone, the driver won't be able to find the volume any more and therefore can't run any volume operations such as clone, attach, delete, etc, but it won't cause any damage to an existing volume either.

[Duncan Thomas (duncan-thomas)]

As Ying says, not really a vulnerability since the provider location is generated by the driver and you need DB write access to change it. Putting in some hardening is no bad thing, but not really exploitable.

[Xing Yang (xing-yang)]

The input is not from a user. When a volume is created, the object path is retrieved from SMI-S and stored in provider_location by the driver. I don't see security vulnerability here.

[Xing Yang (xing-yang)]

Sure, we can add some check there.

[Thierry Carrez (ttx)]

OK, unless someone disagrees, i'll remove the security flag on this one Monday.

[Sean McGinnis (sean-mcginnis)]

Closing stale bug. If this is still an issue please reopen.