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

ewoud at kohlvanwijngaarden.nl ewoud at kohlvanwijngaarden.nl
Mon Apr 23 11:06:55 UTC 2012


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 at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud at kohlvanwijngaarden.nl>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>


More information about the vdsm-patches mailing list