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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <[email protected]> Gerrit-Reviewer: Igor Lvovsky <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
