Merge pull request #754 from nextcloud/bugfix/noid/fix-XML-loading

Fix validating SAML XML schemas

lib/Command/GetMetadata.php

@@ -21,6 +21,7 @@
 
 namespace OCA\User_SAML\Command;
 
+use OCA\User_SAML\Helper\TXmlHelper;
 use OCA\User_SAML\SAMLSettings;
 use OneLogin\Saml2\Error;
 use OneLogin\Saml2\Settings;
@@ -30,6 +31,7 @@
 use Symfony\Component\Console\Output\OutputInterface;
 
 class GetMetadata extends Command {
+	use TXmlHelper;
 
 	/** @var SAMLSettings */
 	private $SAMLSettings;
@@ -71,7 +73,9 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 		$idp = (int)$input->getArgument('idp');
 		$settings = new Settings($this->SAMLSettings->getOneLoginSettingsArray($idp));
 		$metadata = $settings->getSPMetadata();
-		$errors = $settings->validateMetadata($metadata);
+		$errors = $this->callWithXmlEntityLoader(function () use ($settings, $metadata) {
+			return $settings->validateMetadata($metadata);
+		});
 		if (empty($errors)) {
 			$output->writeln($metadata);
 		} else {

lib/Controller/SAMLController.php

@@ -28,6 +28,7 @@
 use OC\Core\Controller\ClientFlowLoginV2Controller;
 use OCA\User_SAML\Exceptions\NoUserFoundException;
 use OCA\User_SAML\Exceptions\UserFilterViolationException;
+use OCA\User_SAML\Helper\TXmlHelper;
 use OCA\User_SAML\SAMLSettings;
 use OCA\User_SAML\UserBackend;
 use OCA\User_SAML\UserData;
@@ -48,6 +49,8 @@
 use OneLogin\Saml2\ValidationError;
 
 class SAMLController extends Controller {
+	use TXmlHelper;
+
 	/** @var ISession */
 	private $session;
 	/** @var IUserSession */
@@ -290,7 +293,9 @@ public function login(int $idp = 1) {
 	public function getMetadata(int $idp = 1) {
 		$settings = new Settings($this->samlSettings->getOneLoginSettingsArray($idp));
 		$metadata = $settings->getSPMetadata();
-		$errors = $settings->validateMetadata($metadata);
+		$errors = $this->callWithXmlEntityLoader(function () use ($settings, $metadata) {
+			return $settings->validateMetadata($metadata);
+		});
 		if (empty($errors)) {
 			return new Http\DataDownloadResponse($metadata, 'metadata.xml', 'text/xml');
 		} else {
@@ -351,7 +356,10 @@ public function assertionConsumerService(): Http\RedirectResponse {
 		}
 
 		$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
-		$auth->processResponse($AuthNRequestID);
+		// validator (called with processResponse()) needs an XML entity loader
+		$this->callWithXmlEntityLoader(function () use ($auth, $AuthNRequestID): void {
+			$auth->processResponse($AuthNRequestID);
+		});
 
 		$this->logger->debug('Attributes send by the IDP: ' . json_encode($auth->getAttributes()));
 
@@ -518,13 +526,16 @@ private function tryProcessSLOResponse(?int $idp): array {
 		foreach ($idps as $idp) {
 			try {
 				$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
-				$targetUrl = $auth->processSLO(
-					true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
-					null,
-					$this->samlSettings->usesSloWebServerDecode($idp),
-					null,
-					true
-				);
+				// validator (called with processSLO()) needs an XML entity loader
+				$targetUrl = $this->callWithXmlEntityLoader(function () use ($auth, $idp): string {
+					return $auth->processSLO(
+						true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
+						null,
+						$this->samlSettings->usesSloWebServerDecode($idp),
+						null,
+						true
+					);
+				});
 				if ($auth->getLastErrorReason() === null) {
 					return [$targetUrl, $auth];
 				}

lib/Helper/TXmlHelper.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Arthur Schiwon <[email protected]>
+ *
+ * @author Arthur Schiwon <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_SAML\Helper;
+
+trait TXmlHelper {
+
+	/**
+	 * @returns mixed returns the result of the callable parameter
+	 */
+	public function callWithXmlEntityLoader(callable $func) {
+		libxml_set_external_entity_loader(static function ($public, $system) {
+			return $system;
+		});
+		$result = $func();
+		libxml_set_external_entity_loader(static function () {
+			return null;
+		});
+		return $result;
+	}
+}