improve mountpoint parsing

changes from v1:
renamed function to verify_*
added check for ../ at the beginning
cleaned up regex (\.)? -> \.?

currently we sanitize mountpoints with sanitize_mountpoint, which
tries to remove dots, double-dots and multiple slashes, but it does it
not correctly (e.g. /test/././ gets truncated to /test./ )

instead of trying to truncate the path, we create a format for mp strings
which throws an error if /./ or /../ exist (also /. and /.. at the end or
../ at the beginning) since there should be no valid use for these in
mountpoint paths anyway

with the new behaviour, we don't need sanitize_mountpoint anymore:

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 6a3489ac14a35c4bfb1cf14a009473ab25db899a..e7330f899fef3eca76db0336528c503e38409b63 100644 (file)
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -38,6 +38,7 @@ my $rootfs_desc = {
     volume => {
        type => 'string',
        default_key => 1,
+       format => 'pve-lxc-mp-string',
        format_description => 'volume',
        description => 'Volume, device or directory to mount into the container.',
     },
@@ -367,10 +368,29 @@ for (my $i = 0; $i < $MAX_LXC_NETWORKS; $i++) {
     };
 }
 
+PVE::JSONSchema::register_format('pve-lxc-mp-string', \&verify_lxc_mp_string);
+sub verify_lxc_mp_string{
+    my ($mp, $noerr) = @_;
+
+    # do not allow:
+    # /./ or /../ 
+    # /. or /.. at the end
+    # ../ at the beginning
+    
+    if($mp =~ m@/\.\.?/@ ||
+       $mp =~ m@/\.\.?$@ ||
+       $mp =~ m@^\.\./@){
+       return undef if $noerr;
+       die "$mp contains illegal character sequences\n";
+    }
+    return $mp;
+}
+
 my $mp_desc = {
     %$rootfs_desc,
     mp => {
        type => 'string',
+       format => 'pve-lxc-mp-string',
        format_description => 'Path',
        description => 'Path to the mountpoint as seen from inside the container.',
     },
@@ -2033,18 +2053,6 @@ 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) = @_;
@@ -2055,11 +2063,6 @@ sub foreach_mountpoint_full {
        my $mountpoint = $key eq 'rootfs' ? parse_ct_rootfs($value, 1) : parse_ct_mountpoint($value, 1);
        next if !defined($mountpoint);
 
-       $mountpoint->{mp} = sanitize_mountpoint($mountpoint->{mp});
-
-       my $path = $mountpoint->{volume};
-       $mountpoint->{volume} = sanitize_mountpoint($path) if $path =~ m|^/|;
-
        &$func($key, $mountpoint);
     }
 }