On 04.11.24 11:42 AM, Fabian Grünbichler wrote:
> creating non-raw disk images with arbitrary content is only possible with raw
> access to the storage, but checking for references to external files doesn't
> hurt.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
> ---
> 
> Notes:
>     requires pve-storage change to actually have an effect
>     
>     v2:
>     - re-order code to improve readability
>     - extract $path into variable to avoid scalar vs array context issue
> 
>  PVE/API2/Qemu.pm | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 848001b6..ae0c39bf 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -412,18 +412,29 @@ my sub create_disks : prototype($$$$$$$$$$) {
>  
>               $needs_creation = $live_import;
>  
> -             if (PVE::Storage::parse_volume_id($source, 1)) { # PVE-managed 
> volume
> +             my ($source_storage, $source_volid) = 
> PVE::Storage::parse_volume_id($source, 1);
> +
> +             if ($source_storage) { # PVE-managed volume
>                   if ($live_import && $ds ne 'efidisk0') {
>                       my $path = PVE::Storage::path($storecfg, $source)
>                           or die "failed to get a path for '$source'\n";
>                       $source = $path;
> -                     ($size, my $source_format) = 
> PVE::Storage::file_size_info($source);
> +                     # check potentially untrusted image file!
> +                     ($size, my $source_format) = 
> PVE::Storage::file_size_info($source, undef, 1);
> +
>                       die "could not get file size of $source\n" if !$size;
>                       $live_import_mapping->{$ds} = {
>                           path => $source,
>                           format => $source_format,
>                       };
>                   } else {
> +                     # check potentially untrusted image file!
> +                     my $scfg = PVE::Storage::storage_config($storecfg, 
> $source_storage);
> +                     if ($scfg->{path}) {

Is there a special reason this is made conditional on the presence of
$scfg->{path}? Note that the above check for live import isn't.
Shouldn't matter much right now (except if there is a weird third-party
plugin that has qcow2 and doesn't use $scfg->{path}), but as long as we
get a result for PVE::Storage::path(), we should be able to call
file_size_info() too, right? Would potentially help detecting other
issues with a source volume early.

> +                         my $path = PVE::Storage::path($storecfg, $source);
> +                         PVE::Storage::file_size_info($path, undef, 1);
> +                     }
> +
>                       my $dest_info = {
>                           vmid => $vmid,
>                           drivename => $ds,
> @@ -441,7 +452,8 @@ my sub create_disks : prototype($$$$$$$$$$) {
>                   }
>               } else {
>                   $source = PVE::Storage::abs_filesystem_path($storecfg, 
> $source, 1);
> -                 ($size, my $source_format) = 
> PVE::Storage::file_size_info($source);
> +                 # check potentially untrusted image file!
> +                 ($size, my $source_format) = 
> PVE::Storage::file_size_info($source, undef, 1);
>                   die "could not get file size of $source\n" if !$size;
>  
>                   if ($live_import && $ds ne 'efidisk0') {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to