[pve-devel] [PATCH pve-container 2/4] use sanitize_mountpoint in foreach_mountpoint

[Wolfgang Bumiller]

This is for consistency as well as it's a minor security
improvement.

If the source is a path we also want to sanitize it in order
to not get broken behavior when using paths like
/dev/../something etc.
---
 src/PVE/LXC.pm        | 22 +++++++++++++++++++++-
 src/PVE/VZDump/LXC.pm | 15 ---------------
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 8abc21e..c7f7ef5 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1818,6 +1818,19 @@ sub mountpoint_names {
     return $reverse ? reverse @names : @names;
 }
 
+# The container might have *different* symlinks than the host. realpath/abs_path
+# use the actual filesystem to resolve links.
+sub sanitize_mountpoint {
+    my ($mp) = @_;
+    $mp = '/' . $mp; # we always start with a slash
+    $mp =~ s@/{2,}@/@g; # collapse sequences of slashes
+    $mp =~ s@/\./@@g; # collapse /./
+    $mp =~ s@/\.(/)?$@$1@; # collapse a trailing /. or /./
+    $mp =~ s@(.*)/[^/]+/\.\./@$1/@g; # collapse /../ without regard for symlinks
+    $mp =~ s@/\.\.(/)?$@$1@; # collapse trailing /.. or /../ disregarding symlinks
+    return $mp;
+}
+
 sub foreach_mountpoint_full {
     my ($conf, $reverse, $func) = @_;
 
@@ -1825,7 +1838,14 @@ sub foreach_mountpoint_full {
 	my $value = $conf->{$key};
 	next if !defined($value);
 	my $mountpoint = parse_ct_mountpoint($value);
-	$mountpoint->{mp} = '/' if $key eq 'rootfs'; # just to be sure
+
+	# just to be sure: rootfs is /
+	my $path = $key eq 'rootfs' ? '/' : $mountpoint->{mp};
+	$mountpoint->{mp} = sanitize_mountpoint($path);
+
+	$path = $mountpoint->{volume};
+	$mountpoint->{volume} = sanitize_mountpoint($path) if $path =~ m|^/|;
+
 	&$func($key, $mountpoint);
     }
 }
diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index 4c421f5..c9a75dd 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -92,19 +92,6 @@ my $check_mountpoint_empty = sub {
     });
 };
 
-# The container might have *different* symlinks than the host. realpath/abs_path
-# use the actual filesystem to resolve links.
-sub sanitize_mountpoint {
-    my ($mp) = @_;
-    $mp = '/' . $mp; # we always start with a slash
-    $mp =~ s@/{2,}@/@g; # collapse sequences of slashes
-    $mp =~ s@/\./@@g; # collapse /./
-    $mp =~ s@/\.(/)?$@$1@; # collapse a trailing /. or /./
-    $mp =~ s@(.*)/[^/]+/\.\./@$1/@g; # collapse /../ without regard for symlinks
-    $mp =~ s@/\.\.(/)?$@$1@; # collapse trailing /.. or /../ disregarding symlinks
-    return $mp;
-}
-
 sub prepare {
     my ($self, $task, $vmid, $mode) = @_;
 
@@ -124,8 +111,6 @@ sub prepare {
 	my $volid = $data->{volume};
 	my $mount = $data->{mp};
 
-	$mount = $data->{mp} = sanitize_mountpoint($mount);
-
 	return if !$volid || !$mount || $volid =~ m|^/|;
 
 	if ($name ne 'rootfs' && !$data->{backup}) {
-- 
2.1.4