From: Noel Ullreich <n.ullreich@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH pve-storage/pve-manager 1/3 v2] recursively go through subdirs to find files
Date: Tue, 16 May 2023 16:51:12 +0200 [thread overview]
Message-ID: <20230516145115.103156-2-n.ullreich@proxmox.com> (raw)
In-Reply-To: <20230516145115.103156-1-n.ullreich@proxmox.com>
This patch allows `get_subdir_files` to recursively call itself, so that
subdirectories of set depth can be searched. We allow searching for
isos, vztmpl and snippets but not backups.
As a security measure, when parsing a given path, parent
directories (`/../`) are forbidden.
The feature is opt-in, i.e. the searchdepth is 0 by default. It can be
changed via the API, the web interface and `pvesm` (see the other
patches).
Signed-off-by: Noel Ullreich <n.ullreich@proxmox.com>
---
PVE/Storage.pm | 7 ++++++
PVE/Storage/Plugin.pm | 54 ++++++++++++++++++++++++++-----------------
2 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index cec3996..0097a53 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -113,6 +113,13 @@ our $VZTMPL_EXT_RE_1 = qr/\.tar\.(gz|xz|zst)/i;
our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/;
+# '..' is forbidden at the beginning, between two '/' and at the end
+my $dots = quotemeta('..');
+my $beginning = qr!^$dots/!;
+my $between = qr!/$dots/!;
+my $end = qr!/$dots$!;
+our $forbidden_double_dots_re = qr!(?:$beginning|$between|$end)!;
+
# FIXME remove with PVE 8.0, add versioned breaks for pve-manager
our $vztmpl_extension_re = $VZTMPL_EXT_RE_1;
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index c323085..e5b53b0 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -614,6 +614,8 @@ sub parse_name_dir {
sub parse_volname {
my ($class, $volname) = @_;
+ die "volname must not contain parent directories '/../'\n" if $volname =~ $PVE::Storage::forbidden_double_dots_re;
+
if ($volname =~ m!^(\d+)/(\S+)/(\d+)/(\S+)$!) {
my ($basedvmid, $basename) = ($1, $2);
parse_name_dir($basename);
@@ -624,9 +626,9 @@ sub parse_volname {
my ($vmid, $name) = ($1, $2);
my (undef, $format, $isBase) = parse_name_dir($name);
return ('images', $name, $vmid, undef, undef, $isBase, $format);
- } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) {
+ } elsif ($volname =~ m!^iso/((?:[0-9A-z\_\-\.]+\/)*[^\/]+$PVE::Storage::ISO_EXT_RE_0)$!) {
return ('iso', $1);
- } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) {
+ } elsif ($volname =~ m!^vztmpl/((?:[0-9A-z\_\-\.]+\/)*[^\/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) {
return ('vztmpl', $1);
} elsif ($volname =~ m!^rootdir/(\d+)$!) {
return ('rootdir', $1, $1);
@@ -636,7 +638,7 @@ sub parse_volname {
return ('backup', $fn, $2);
}
return ('backup', $fn);
- } elsif ($volname =~ m!^snippets/([^/]+)$!) {
+ } elsif ($volname =~ m!^snippets/((?:[0-9A-z\_\-\.]+\/)*[^\/]+)$!) {
return ('snippets', $1);
}
@@ -1205,28 +1207,33 @@ sub list_images {
}
# list templates ($tt = <iso|vztmpl|backup|snippets>)
-my $get_subdir_files = sub {
- my ($sid, $path, $tt, $vmid) = @_;
+sub get_subdir_files {
+ my ($sid, $path, $tt, $scfg, $vmid, $remaining_depth) = @_;
+ my $storage_path = $scfg->{path};
+ my $content_dir = $scfg->{"content-dirs"}->{$tt} // $vtype_subdirs->{$tt};
my $res = [];
foreach my $fn (<$path/*>) {
- my $st = File::stat::stat($fn);
+ my $st = File::stat::lstat($fn);
+
+ next if (!$st);
- next if (!$st || S_ISDIR($st->mode));
+ if (S_ISDIR($st->mode)) {
+ if ($remaining_depth) {
+ push @$res, get_subdir_files($sid, $fn, $tt, $scfg, $vmid, $remaining_depth-1);
+ }
+ next;
+ }
my $info;
if ($tt eq 'iso') {
- next if $fn !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i;
-
+ next if $fn !~ m/(?:^$storage_path\/$content_dir\/)((?:[0-9A-z\_\-\.]+\/)*[^\/]+$PVE::Storage::ISO_EXT_RE_0)/;
$info = { volid => "$sid:iso/$1", format => 'iso' };
-
} elsif ($tt eq 'vztmpl') {
- next if $fn !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!;
-
+ next if $fn !~ m/(?:^$storage_path\/$content_dir\/)((?:[0-9A-z\_\-\.]+\/)*[^\/]+$PVE::Storage::VZTMPL_EXT_RE_1)/;
$info = { volid => "$sid:vztmpl/$1", format => "t$2" };
-
} elsif ($tt eq 'backup') {
next if $fn !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!;
my $original = $fn;
@@ -1255,7 +1262,7 @@ my $get_subdir_files = sub {
$info->{protected} = 1 if -e PVE::Storage::protection_file_path($original);
} elsif ($tt eq 'snippets') {
-
+ next if $fn !~ m/(?:^$storage_path\/$content_dir\/)((?:[0-9A-z\_\-\.]+\/)*.+)/;
$info = {
volid => "$sid:snippets/". basename($fn),
format => 'snippet',
@@ -1267,14 +1274,18 @@ my $get_subdir_files = sub {
push @$res, $info;
}
-
return $res;
};
+sub flatten {
+ map { ref eq 'ARRAY' ? flatten(@{$_}) : $_ } @_;
+}
+
# If attributes are set on a volume, they should be included in the result.
# See get_volume_attribute for a list of possible attributes.
sub list_volumes {
my ($class, $storeid, $scfg, $vmid, $content_types) = @_;
+ my $max_depth = $scfg->{'subdir-depth'} // 0;
my $res = [];
my $vmlist = PVE::Cluster::get_vmlist();
@@ -1287,17 +1298,19 @@ sub list_volumes {
my $path = $class->get_subdir($scfg, $type);
if ($type eq 'iso' && !defined($vmid)) {
- $data = $get_subdir_files->($storeid, $path, 'iso');
+ $data = get_subdir_files($storeid, $path, 'iso', $scfg, undef, $max_depth);
} elsif ($type eq 'vztmpl'&& !defined($vmid)) {
- $data = $get_subdir_files->($storeid, $path, 'vztmpl');
+ $data = get_subdir_files($storeid, $path , 'vztmpl', $scfg, undef, $max_depth);
} elsif ($type eq 'backup') {
- $data = $get_subdir_files->($storeid, $path, 'backup', $vmid);
+ $data = get_subdir_files($storeid, $path, 'backup', $scfg, $vmid, $max_depth);
} elsif ($type eq 'snippets') {
- $data = $get_subdir_files->($storeid, $path, 'snippets');
+ $data = get_subdir_files($storeid, $path, 'snippets', $scfg, undef, $max_depth);
}
}
- next if !$data;
+ $data = [flatten($data)];
+
+ next if !@$data[0];
foreach my $item (@$data) {
if ($type eq 'images' || $type eq 'rootdir') {
@@ -1315,7 +1328,6 @@ sub list_volumes {
} else {
$item->{content} = $type;
}
-
push @$res, $item;
}
}
--
2.30.2
next prev parent reply other threads:[~2023-05-16 14:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-16 14:51 [pve-devel] [PATCH pve-storage/pve-manager 0/3 v2] fix #623: show isos/vztmpl/snippets in subdirs Noel Ullreich
2023-05-16 14:51 ` Noel Ullreich [this message]
2023-05-16 14:51 ` [pve-devel] [PATCH pve-storage/pve-manager 2/3 v2] add `subdir-depth` option to filesystems Noel Ullreich
2023-05-16 14:51 ` [pve-devel] [PATCH pve-storage/pve-manager 3/3 v2] update test for recursive subdir search Noel Ullreich
2023-05-16 14:51 ` [pve-devel] [PATCH pve-storage/pve-manager 1/1 v2] add field to set subdir-depth in web interface Noel Ullreich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230516145115.103156-2-n.ullreich@proxmox.com \
--to=n.ullreich@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.