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 116F41FF16B for ; Thu, 14 Nov 2024 10:34:16 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 159CB2C896; Thu, 14 Nov 2024 10:33:00 +0100 (CET) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Thu, 14 Nov 2024 10:32:05 +0100 Message-Id: <20241114093226.814530-5-d.csapak@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20241114093226.814530-1-d.csapak@proxmox.com> References: <20241114093226.814530-1-d.csapak@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.014 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [ovf.pm] Subject: [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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" moves the filepath code a bit more closer to where it's actually used checks the contained path before trying to find it's absolute path properly add error handling to realpath instead of checking the combined ovf_path + filepath, just make sure filepath can't point to anythign besides a file in this directory by checking for '.' and '..' (slashes are not allowed in SAFE_CHAR_CLASS_RE) Signed-off-by: Dominik Csapak --- src/PVE/GuestImport/OVF.pm | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm index c7bff5f..77dba14 100644 --- a/src/PVE/GuestImport/OVF.pm +++ b/src/PVE/GuestImport/OVF.pm @@ -220,15 +220,6 @@ ovf:Disk[\@ovf:diskId='%s']/\@ovf:capacityAllocationUnits", $disk_id); next; } - # 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; - # from Item, find owning Controller type my $controller_id = $xpc->findvalue('rasd:Parent', $item_node); my $xpath_find_parent_type = sprintf("/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/\ @@ -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"; - } + 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" -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel