Skip to content

Commit 44d4187

Browse files
committed
Verify signed info first before using it
Thanks @ahacker1-securesaml for reporting this
1 parent 40daae0 commit 44d4187

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

src/XML/SignedElementTrait.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,17 +201,20 @@ private function verifyInternal(SignatureAlgorithmInterface $verifier): SignedEl
201201

202202
// the canonicalized ds:SignedInfo element (plaintext)
203203
$c14nSignedInfo = $signedInfo->canonicalize($c14nAlg->getValue());
204-
$ref = $this->validateReference(
205-
SignedInfo::fromXML(DOMDocumentFactory::fromString($c14nSignedInfo)->documentElement),
206-
);
207204

205+
// verify the c14n signed info has correct signature
208206
if (
209207
$verifier?->verify(
210208
$c14nSignedInfo, // the canonicalized ds:SignedInfo element (plaintext)
211209
// the actual signature
212210
base64_decode(strval($this->getSignature()->getSignatureValue()->getValue()), true),
213211
)
214212
) {
213+
// uses the trusted c14nsignedinfo and only the c14nsignedinfo to validate references
214+
$ref = $this->validateReference(
215+
SignedInfo::fromXML(DOMDocumentFactory::fromString($c14nSignedInfo)->documentElement),
216+
);
217+
215218
/*
216219
* validateReference() returns an object of the same class using this trait. This means the validatingKey
217220
* property is available, and we can set it on the newly created object because we are in the same class,
@@ -220,6 +223,7 @@ private function verifyInternal(SignatureAlgorithmInterface $verifier): SignedEl
220223
$ref->validatingKey = $verifier->getKey();
221224
return $ref;
222225
}
226+
223227
throw new SignatureVerificationFailedException('Failed to verify signature.');
224228
}
225229

0 commit comments

Comments
 (0)