From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 609D21FF16F
	for <inbox@lore.proxmox.com>; Fri, 15 Nov 2024 14:35:32 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 806A3144F3;
	Fri, 15 Nov 2024 14:35:34 +0100 (CET)
Message-ID: <d35e4213-2597-45c2-84b3-352e8431abe2@proxmox.com>
Date: Fri, 15 Nov 2024 14:35:01 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Dominik Csapak <d.csapak@proxmox.com>
References: <20241114093226.814530-1-d.csapak@proxmox.com>
 <20241114093226.814530-5-d.csapak@proxmox.com>
Content-Language: en-GB
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <20241114093226.814530-5-d.csapak@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.055 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH storage v5 04/12] ovf: improve and simplify
 path checking code
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

On 14.11.24 10:32 AM, Dominik Csapak wrote:
> @@ -244,22 +235,31 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
>  	my $adress_on_controller = $xpc->findvalue('rasd:AddressOnParent', $item_node);
>  	my $pve_disk_address = id_to_pve($controller_type) . $adress_on_controller;
>  
> +	# from Disk Node, find corresponding filepath
> +	my $xpath_find_filepath = sprintf("/ovf:Envelope/ovf:References/ovf:File[\@ovf:id='%s']/\@ovf:href", $fileref);
> +	my $filepath = $xpc->findvalue($xpath_find_filepath);
> +	if (!$filepath) {
> +	    warn "invalid file reference $fileref, skipping\n";
> +	    next;
> +	}
> +	print "file path: $filepath\n" if $debug;
> +	my $original_filepath = $filepath;
> +	($filepath) = $filepath =~ m|^(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$|; # untaint & check no sub/parent dirs
> +	die "referenced path '$original_filepath' is invalid\n" if !$filepath || $filepath eq "." || $filepath eq "..";
> +
>  	# resolve symlinks and relative path components
>  	# and die if the diskimage is not somewhere under the $ovf path
> -	my $ovf_dir = realpath(dirname(File::Spec->rel2abs($ovf)));
> -	my $backing_file_path = realpath(join ('/', $ovf_dir, $filepath));
> -	if ($backing_file_path !~ /^\Q${ovf_dir}\E/) {
> -	    die "error parsing $filepath, are you using a symlink ?\n";
> -	}

Don't we still need this check against symlinks?

> +	my $ovf_dir = realpath(dirname(File::Spec->rel2abs($ovf)))
> +	    or die "could not get absolute path of $ovf: $!\n";
> +	my $backing_file_path = realpath(join ('/', $ovf_dir, $filepath))
> +	    or die "could not get absolute path of $filepath: $!\n";
> +
> +	($backing_file_path) = $backing_file_path =~ m|^(/.*)|; # untaint
>  
>  	if (!-e $backing_file_path && !$isOva) {
>  	    die "error parsing $filepath, file seems not to exist at $backing_file_path\n";
>  	}
>  
> -	($backing_file_path) = $backing_file_path =~ m|^(/.*)|; # untaint
> -	($filepath) = $filepath =~ m|^(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$|; # untaint & check no sub/parent dirs
> -	die "invalid path\n" if !$filepath;
> -
>  	if (!$isOva) {
>  	    my $size = PVE::Storage::file_size_info($backing_file_path);
>  	    die "error parsing $backing_file_path, cannot determine file size\n"



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