From 6c70a5e66caaf05685702ff250b2eb403d275b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:59:56 -0300 Subject: [PATCH 1/2] fix(xcatd): block XML external entities on the legacy parser path xCAT::XML wraps XML::Simple. It blocks XML external entities. An external entity can read a local file or a network resource. XML::Simple with new_xml_parser() installs an ExternEnt handler. The handler returns the system id, not the content. XML::Simple 2.20 to 2.24 has no new_xml_parser(). On these versions xCAT uses build_tree_xml_parser() instead. This function did not install the handler. A SYSTEM entity then expanded. It leaked a local file into the parsed data. This change adds the same handler to build_tree_xml_parser(). Both paths pass parser options in an array reference. XML::Parser->new ignores that reference. The handler is therefore the real defense. This fix keeps #6917, which retains the compatibility path for those releases. This fix was recovered from the lenovobuild branch. See PR #6505. --- xCAT-server/lib/perl/xCAT/XML.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/xCAT-server/lib/perl/xCAT/XML.pm b/xCAT-server/lib/perl/xCAT/XML.pm index 972b331d2..9c3a2afdc 100644 --- a/xCAT-server/lib/perl/xCAT/XML.pm +++ b/xCAT-server/lib/perl/xCAT/XML.pm @@ -56,6 +56,7 @@ sub build_tree_xml_parser { no_network => 1, expand_entities => 0, ]); + $xp->setHandlers(ExternEnt => sub { return $_[2] }); my($tree); if($filename) { # $tree = $xp->parsefile($filename); # Changed due to prob w/mod_perl From 467d8182c47908a5f5e8977aa7d024327eddf78f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:59:57 -0300 Subject: [PATCH 2/2] test(xcatd): cover XML external-entity suppression The test sends a document whose external SYSTEM entity points at a secret file. It exercises both parser paths: the modern XML::Simple path and the older compatibility path. Each path must parse the document, replace the entity with its system identifier, and not read the file contents. --- xCAT-test/unit/xml_external_entity.t | 72 ++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 xCAT-test/unit/xml_external_entity.t diff --git a/xCAT-test/unit/xml_external_entity.t b/xCAT-test/unit/xml_external_entity.t new file mode 100644 index 000000000..02f68605c --- /dev/null +++ b/xCAT-test/unit/xml_external_entity.t @@ -0,0 +1,72 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use File::Temp qw(tempfile); +use Test::More; + +my $libdir = "$FindBin::Bin/../../xCAT-server/lib/perl"; +my $xmlpm = "$libdir/xCAT/XML.pm"; +plan skip_all => 'xCAT::XML not found' unless -r $xmlpm; +eval { require XML::Simple; require XML::Parser; 1 } + or plan skip_all => 'XML::Simple and XML::Parser are required'; + +# xCAT::XML loads xCAT::MsgUtils, which loads much of the xCAT tree. The parser +# paths never call it. Stub it before loading xCAT::XML. +BEGIN { $INC{'xCAT/MsgUtils.pm'} = 1; } +{ package xCAT::MsgUtils; } + +# Prepend so the source-tree module wins over any installed xCAT::XML. +unshift @INC, $libdir; +require xCAT::XML; +require Data::Dumper; + +my ($sfh, $secret_path) = tempfile('xcat_xxe_XXXXXX', TMPDIR => 1, UNLINK => 1); +print {$sfh} "SECRET-CONTENT-DO-NOT-LEAK"; +close $sfh; + +my $payload = <<"XML"; + + ]> +&xxe; +XML + +sub parsed_tree { + my $parser = xCAT::XML->new; + my $tree = eval { $parser->XMLin($payload, SuppressEmpty => undef, ForceArray => 1) }; + return ($tree, $@); +} + +# A parser path must parse the payload, replace the external entity with its +# system identifier, and never read the file contents. +sub check_path { + my ($label) = @_; + my ($tree, $error) = parsed_tree(); + is($error, '', "$label: the payload parses without error"); + ok(defined($tree), "$label: the parser returns a tree"); + my $dump = defined($tree) ? Data::Dumper::Dumper($tree) : ''; + like($dump, qr{\Q$secret_path\E}, + "$label: the external entity is replaced by its system identifier"); + unlike($dump, qr/SECRET-CONTENT-DO-NOT-LEAK/, + "$label: the external entity content is not read"); +} + +# The modern path: XML::Simple with new_xml_parser. +SKIP: { + skip 'XML::Simple lacks new_xml_parser on this system', 4 + unless exists &{'XML::Simple::new_xml_parser'}; + check_path('modern path'); +} + +# Force the older compatibility path (build_tree_xml_parser's own code) by +# removing new_xml_parser, as on XML::Simple 2.20-2.24. +{ + no strict 'refs'; + no warnings 'redefine'; + undef *{'XML::Simple::new_xml_parser'} if exists &{'XML::Simple::new_xml_parser'}; +} +ok(!exists &{'XML::Simple::new_xml_parser'}, 'compatibility path is forced'); +check_path('compatibility path'); + +done_testing();