On Thu, Feb 04, 2016 at 01:40:15PM +0100, Dominik Csapak wrote:
> 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.csa...@proxmox.com>
Acked-by: Wolfgang Bumiller <w.bumil...@proxmox.com>
Please write the patch changelog after the '---'-line below next time
so they don't get included in the commit message when applying with
git-am.
> ---
> src/PVE/LXC.pm | 37 ++++++++++++++++++++-----------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index f761f33..0ea30f2 100644
> --- 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.',
> },
> @@ -2024,18 +2044,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) = @_;
> @@ -2046,11 +2054,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);
> }
> }
> --
> 2.1.4
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
