From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 07D5C1FF16F for ; Fri, 15 Nov 2024 14:39:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3217A14694; Fri, 15 Nov 2024 14:39:37 +0100 (CET) Message-ID: <9eb64fa0-7add-4994-8419-1310978e2b8e@proxmox.com> Date: Fri, 15 Nov 2024 14:39:33 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Fiona Ebner , Proxmox VE development discussion References: <20241114093226.814530-1-d.csapak@proxmox.com> <20241114093226.814530-5-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On 11/15/24 14:35, Fiona Ebner wrote: > 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? yeah i think you're right, but only in the ovf case so I'd add it... > >> + 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) { here ? >> 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