Change in vdsm[master]: vm payload: add file injection to vm

Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: vm payload: add file injection to vm
......................................................................


Patch Set 19: (2 inline comments)

....................................................
File vdsm/mkimage.py
Line 32: PAYLOAD_IMAGES_P = '/tmp'
I think /var/run is a bad place for this. FHS 3.0 draft 1 about /var/run
(http://www.linuxbase.org/betaspecs/fhs/fhs/ch05s13.html):

This directory was once intended for system information data describing the system since
it was booted. These functions have been moved to /run; this directory exists to ensure
compatibility with systems and software using an older version of this specification.

Then we have /run (http://www.linuxbase.org/betaspecs/fhs/fhs/ch03s15.html):

This directory contains system information data describing the system since it was booted.
Files under this directory must be cleared (removed or truncated as appropriate) at the
beginning of the boot process.

So it's going away in favor of /run and file payloads don't describe the system
state.

Line 106:     if not path.startswith(PAYLOAD_IMAGES_P):
This is a string and string comparisons on paths is still not very secure. Consider
/tmp/../etc/passwd for example.

--
To view, visit http://gerrit.ovirt.org/2321
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I256475342c79690a95ad999335522f99714cdc8b
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud(a)kohlvanwijngaarden.nl&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com&gt;