yesBad/pve-storage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

file_size_info: implement untrusted mode

Browse Source 
this allows checking some extra attributes for images which come from
a potentially malicious source.

since file_size_info is not part of the plugin API, no API bump is
needed. if desired, a similar check could also be implemented in
volume_size_info, which would entail bumping both APIVER and APIAGE
(since the additional parameter would make checking untrusted volumes
opt-in for external plugins).

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
This commit is contained in:
Fabian Grünbichler
2024-11-04 11:42:20 +01:00
committed by Thomas Lamprecht
parent 1c0ebbaae5
commit bffcbe2662
2 changed files with 33 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/PVE/Storage.pm
View File

@ -233,9 +233,9 @@ sub storage_ids {
} }
sub file_size_info { sub file_size_info {
my ($filename, $timeout) = @_; my ($filename, $timeout, $untrusted) = @_;
return PVE::Storage::Plugin::file_size_info($filename, $timeout); return PVE::Storage::Plugin::file_size_info($filename, $timeout, $untrusted);
} }
sub get_volume_attribute { sub get_volume_attribute {

32
src/PVE/Storage/Plugin.pm
View File

@ -943,16 +943,26 @@ sub free_image {
return undef; return undef;
} }
# set $untrusted if the file in question might be malicious since it isn't
# created by our stack
# this makes certain checks fatal, and adds extra checks for known problems like
# - backing files (qcow2/vmdk)
# - external data files (qcow2)
sub file_size_info { sub file_size_info {
my ($filename, $timeout) = @_; my ($filename, $timeout, $untrusted) = @_;
my $st = File::stat::stat($filename); my $st = File::stat::stat($filename);
if (!defined($st)) { if (!defined($st)) {
my $extramsg = -l $filename ? ' - dangling symlink?' : ''; my $extramsg = -l $filename ? ' - dangling symlink?' : '';
warn "failed to stat '$filename'$extramsg\n"; my $msg = "failed to stat '$filename'$extramsg\n";
if ($untrusted) {
die $msg;
} else {
warn $msg;
return undef; return undef;
} }
}
if (S_ISDIR($st->mode)) { if (S_ISDIR($st->mode)) {
return wantarray ? (0, 'subvol', 0, undef, $st->ctime) : 1; return wantarray ? (0, 'subvol', 0, undef, $st->ctime) : 1;
@ -975,18 +985,34 @@ sub file_size_info {
warn $err_output; warn $err_output;
} }
if (!$json) { if (!$json) {
die "failed to query file information with qemu-img\n" if $untrusted;
# skip decoding if there was no output, e.g. if there was a timeout. # skip decoding if there was no output, e.g. if there was a timeout.
return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef; return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef;
} }
my $info = eval { decode_json($json) }; my $info = eval { decode_json($json) };
if (my $err = $@) { if (my $err = $@) {
warn "could not parse qemu-img info command output for '$filename' - $err\n"; my $msg = "could not parse qemu-img info command output for '$filename' - $err\n";
if ($untrusted) {
die $msg;
} else {
warn $msg;
return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef; return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef;
} }
}
if ($untrusted) {
if (my $format_specific = $info->{'format-specific'}) {
if ($format_specific->{type} eq 'qcow2' && $format_specific->{data}->{"data-file"}) {
die "$filename: 'data-file' references are not allowed!\n";
}
}
}
my ($size, $format, $used, $parent) = $info->@{qw(virtual-size format actual-size backing-filename)}; my ($size, $format, $used, $parent) = $info->@{qw(virtual-size format actual-size backing-filename)};
die "backing file not allowed for untrusted image '$filename'!\n" if $untrusted && $parent;
($size) = ($size =~ /^(\d+)$/); # untaint ($size) = ($size =~ /^(\d+)$/); # untaint
die "size '$size' not an integer\n" if !defined($size); die "size '$size' not an integer\n" if !defined($size);
# coerce back from string # coerce back from string