CVE-2023-49087: Insufficient Verification of Data Authenticity (CWE-345)
value: xml-security is a library that implements XML signatures and encryption. Validation of an XML signature requires verification that the hash value of the related XML-document matches a specific DigestValue-value, but also that the cryptographic signature on the SignedInfo-tree (the one that contains the DigestValue) verifies and matches a trusted public key. If an attacker somehow (i.e. by exploiting a bug in PHP's canonicalization function) manages to manipulate the canonicalized version's DigestValue, it would be possible to forge the signature. This issue has been patched in version 1.6.12 and 5.0.0-alpha.13.
CWE description: The product does not sufficiently verify the origin or authenticity of data, in a way that causes it to accept invalid data.
CVSS v3 base score: 7.5
Published: 2023-11-30T06:15Z
Repository: https://github.com/simplesamlphp/xml-security
Language: PHP
Files changed: src/XML/SignedElementTrait.php
Commit date: 2023-11-27 19:17:51 +0100
Commit message: Merge pull request from GHSA-ww7x-3gxh-qm6r


Diff: diff --git a/src/XML/SignedElementTrait.php b/src/XML/SignedElementTrait.php
index 59e7b92..c595a1e 100644
--- a/src/XML/SignedElementTrait.php
+++ b/src/XML/SignedElementTrait.php
@@ -22,10 +22,11 @@ use SimpleSAML\XMLSecurity\Key;
 use SimpleSAML\XMLSecurity\Utils\Security;
 use SimpleSAML\XMLSecurity\Utils\XML;
 use SimpleSAML\XMLSecurity\Utils\XPath;
 use SimpleSAML\XMLSecurity\XML\ds\Reference;
 use SimpleSAML\XMLSecurity\XML\ds\Signature;
+use SimpleSAML\XMLSecurity\XML\ds\SignedInfo;
 use SimpleSAML\XMLSecurity\XML\ds\X509Certificate;
 use SimpleSAML\XMLSecurity\XML\ds\X509Data;
 
 use function array_pop;
 use function base64_decode;
@@ -122,16 +123,15 @@ trait SignedElementTrait
         }
     }
 
 
     /**
+     * @param \SimpleSAML\XMLSecurity\XML\ds\SignedInfo $signedInfo
      * @return \SimpleSAML\XMLSecurity\XML\SignedElementInterface
      */
-    private function validateReference(): SignedElementInterface
+    private function validateReference(SignedInfo $signedInfo): SignedElementInterface
     {
-        /** @var \SimpleSAML\XMLSecurity\XML\ds\Signature $this->signature */
-        $signedInfo = $this->signature->getSignedInfo();
         $references = $signedInfo->getReferences();
         Assert::count(
             $references,
             1,
             'Exactly one reference expected in signature.',
@@ -175,12 +175,16 @@ trait SignedElementTrait
     private function verifyInternal(SignatureAlgorithmInterface $verifier): SignedElementInterface
     {
         /** @var \SimpleSAML\XMLSecurity\XML\ds\Signature $this->signature */
         $signedInfo = $this->signature->getSignedInfo();
         $c14nAlg = $signedInfo->getCanonicalizationMethod()->getAlgorithm();
+
+        // the canonicalized ds:SignedInfo element (plaintext)
         $c14nSignedInfo = $signedInfo->canonicalize($c14nAlg);
-        $ref = $this->validateReference();
+        $ref = $this->validateReference(
+            SignedInfo::fromXML(DOMDocumentFactory::fromString($c14nSignedInfo)->documentElement),
+        );
 
         if (
             $verifier?->verify(
                 $c14nSignedInfo, // the canonicalized ds:SignedInfo element (plaintext)
                 base64_decode($this->signature->getSignatureValue()->getRawContent()), // the actual signature

Vulnerable code:     private function validateReference(): SignedElementInterface
        /** @var \SimpleSAML\XMLSecurity\XML\ds\Signature $this->signature */
        $signedInfo = $this->signature->getSignedInfo();
        $ref = $this->validateReference();
Fixed code: use SimpleSAML\XMLSecurity\XML\ds\SignedInfo;
     * @param \SimpleSAML\XMLSecurity\XML\ds\SignedInfo $signedInfo
    private function validateReference(SignedInfo $signedInfo): SignedElementInterface

        // the canonicalized ds:SignedInfo element (plaintext)
        $ref = $this->validateReference(
            SignedInfo::fromXML(DOMDocumentFactory::fromString($c14nSignedInfo)->documentElement),
        );