File CVE-2025-31335.patch of Package opensaml.38378
From 22a610b322e2178abd03e97cdbc8fb50b45efaee Mon Sep 17 00:00:00 2001
From: Scott Cantor <cantor.2@osu.edu>
Date: Thu, 13 Mar 2025 14:14:12 -0400
Subject: [PATCH 1/1] CPPOST-126 - Simple signature verification fails to
detect parameter smuggling
https://shibboleth.atlassian.net/browse/CPPOST-126
---
saml/binding/impl/SimpleSigningRule.cpp | 88 +++++++++++++------
.../binding/impl/SAML2ArtifactDecoder.cpp | 2 +
saml/saml2/binding/impl/SAML2ECPDecoder.cpp | 3 +-
saml/saml2/binding/impl/SAML2POSTDecoder.cpp | 31 ++++---
.../binding/impl/SAML2RedirectDecoder.cpp | 32 ++++---
saml/saml2/binding/impl/SAML2SOAPDecoder.cpp | 2 +-
6 files changed, 110 insertions(+), 48 deletions(-)
Index: opensaml-2.5.5/saml/binding/impl/SimpleSigningRule.cpp
===================================================================
--- opensaml-2.5.5.orig/saml/binding/impl/SimpleSigningRule.cpp
+++ opensaml-2.5.5/saml/binding/impl/SimpleSigningRule.cpp
@@ -29,6 +29,7 @@
#include "binding/SecurityPolicy.h"
#include "binding/SecurityPolicyRule.h"
#include "saml2/core/Assertions.h"
+#include "saml2/core/Protocols.h"
#include "saml2/metadata/Metadata.h"
#include "saml2/metadata/MetadataCredentialCriteria.h"
#include "saml2/metadata/MetadataProvider.h"
@@ -41,6 +42,7 @@
#include <xmltooling/signature/KeyInfo.h>
#include <xmltooling/signature/Signature.h>
#include <xmltooling/util/ParserPool.h>
+#include <xmltooling/util/URLEncoder.h>
using namespace opensaml::saml2md;
using namespace opensaml;
@@ -65,7 +67,8 @@ namespace opensaml {
private:
// Appends a raw parameter=value pair to the string.
- static bool appendParameter(string& s, const char* data, const char* name);
+ static bool appendParameter(const GenericRequest& request, string& s, const char* data, const char* name);
+ static const char* getMessageParameterName(const XMLObject* message);
bool m_errorFatal;
};
@@ -78,21 +81,48 @@ namespace opensaml {
static const XMLCh errorFatal[] = UNICODE_LITERAL_10(e,r,r,o,r,F,a,t,a,l);
};
-bool SimpleSigningRule::appendParameter(string& s, const char* data, const char* name)
+bool SimpleSigningRule::appendParameter(const GenericRequest& request, string& s, const char* data, const char* name)
{
- const char* start = strstr(data,name);
+ // Make sure only a single instance of the parameter specified is found in the decoded query.
+ vector<const char*> valueHolder;
+ if (request.getParameters(name, valueHolder) > 1) {
+ throw SecurityPolicyException("Multiple $1 parameters present.", params(1, name));
+ }
+
+ string param_name(name);
+ param_name += '=';
+
+ const char* start = strstr(data, param_name.c_str());
if (!start)
return false;
+ if (start > data && *(start - 1) != '&')
+ throw SecurityPolicyException("Detected attempt to smuggle a prefixed $1 parameter.", params(1, name));
+
if (!s.empty())
s += '&';
- const char* end = strchr(start,'&');
+
+ const char* end = strchr(start, '&');
if (end)
- s.append(start, end-start);
+ s.append(start, end - start);
else
s.append(start);
+
return true;
}
+const char* SimpleSigningRule::getMessageParameterName(const XMLObject* message)
+{
+ if (dynamic_cast<const saml2p::StatusResponseType*>(message)) {
+ return "SAMLResponse";
+ }
+ else if (dynamic_cast<const saml2p::RequestAbstractType*>(message)) {
+ return "SAMLRequest";
+ }
+ else {
+ return nullptr;
+ }
+}
+
SimpleSigningRule::SimpleSigningRule(const DOMElement* e) : m_errorFatal(XMLHelper::getAttrBool(e, false, errorFatal))
{
}
@@ -113,34 +143,50 @@ bool SimpleSigningRule::evaluate(const X
}
const HTTPRequest* httpRequest = dynamic_cast<const HTTPRequest*>(request);
- if (!request || !httpRequest)
+ if (!request || !httpRequest) {
return false;
+ }
- const char* signature = request->getParameter("Signature");
- if (!signature)
+ // Make sure Signature only shows up once.
+ vector<const char*> valueHolder;
+ request->getParameters("Signature", valueHolder);
+ if (valueHolder.empty()) {
return false;
-
+ }
+ else if (valueHolder.size() > 1) {
+ throw SecurityPolicyException("Multiple Signature parameters present.");
+ }
+ const char* signature = valueHolder[0];
+
+ // The multiple parameter copy check for the GET case is applied down below in appendParameter.
const char* sigAlgorithm = request->getParameter("SigAlg");
if (!sigAlgorithm) {
log.error("SigAlg parameter not found, no way to verify the signature");
return false;
}
+ const char* messageParameterName = getMessageParameterName(&message);
+ if (!messageParameterName) {
+ log.debug("ignoring unrecognized message type");
+ return false;
+ }
+
string input;
const char* pch;
if (!strcmp(httpRequest->getMethod(), "GET")) {
// We have to construct a string containing the signature input by accessing the
// request directly. We can't use the decoded parameters because we need the raw
- // data and URL-encoding isn't canonical.
+ // data and URL-encoding isn't canonical. We have to ensure only one copy a given
+ // parameter appears in the string in its decoded form, to ensure that other layers
+ // of the code only saw/see the same value we see here.
// NOTE: SimpleSign for GET means Redirect binding, which means we verify over the
// base64-encoded message directly.
pch = httpRequest->getQueryString();
- if (!appendParameter(input, pch, "SAMLRequest="))
- appendParameter(input, pch, "SAMLResponse=");
- appendParameter(input, pch, "RelayState=");
- appendParameter(input, pch, "SigAlg=");
+ appendParameter(*request, input, pch, messageParameterName);
+ appendParameter(*request, input, pch, "RelayState");
+ appendParameter(*request, input, pch, "SigAlg");
}
else {
// With POST, the input string is concatenated from the decoded form controls.
@@ -152,28 +198,14 @@ bool SimpleSigningRule::evaluate(const X
// why XMLSignature exists, and why this isn't really "simpler").
xsecsize_t x;
- pch = httpRequest->getParameter("SAMLRequest");
+ pch = httpRequest->getParameter(messageParameterName);
if (pch) {
XMLByte* decoded=Base64::decode(reinterpret_cast<const XMLByte*>(pch),&x);
if (!decoded) {
log.warn("unable to decode base64 in POST binding message");
return false;
}
- input = string("SAMLRequest=") + reinterpret_cast<const char*>(decoded);
-#ifdef OPENSAML_XERCESC_HAS_XMLBYTE_RELEASE
- XMLString::release(&decoded);
-#else
- XMLString::release((char**)&decoded);
-#endif
- }
- else {
- pch = httpRequest->getParameter("SAMLResponse");
- XMLByte* decoded=Base64::decode(reinterpret_cast<const XMLByte*>(pch),&x);
- if (!decoded) {
- log.warn("unable to decode base64 in POST binding message");
- return false;
- }
- input = string("SAMLResponse=") + reinterpret_cast<const char*>(decoded);
+ input = string(messageParameterName) + "=" + reinterpret_cast<const char*>(decoded);
#ifdef OPENSAML_XERCESC_HAS_XMLBYTE_RELEASE
XMLString::release(&decoded);
#else
Index: opensaml-2.5.5/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp
===================================================================
--- opensaml-2.5.5.orig/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp
+++ opensaml-2.5.5/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp
@@ -92,6 +92,8 @@ XMLObject* SAML2ArtifactDecoder::decode(
const char* state = httpRequest->getParameter("RelayState");
if (state)
relayState = state;
+ if (httpRequest->getParameter("Signature"))
+ throw BindingException("Request contained unexpected Signature parameter.");
if (!m_artifactResolver || !policy.getMetadataProvider() || !policy.getRole())
throw BindingException("Artifact binding requires ArtifactResolver and MetadataProvider implementations be supplied.");
Index: opensaml-2.5.5/saml/saml2/binding/impl/SAML2ECPDecoder.cpp
===================================================================
--- opensaml-2.5.5.orig/saml/saml2/binding/impl/SAML2ECPDecoder.cpp
+++ opensaml-2.5.5/saml/saml2/binding/impl/SAML2ECPDecoder.cpp
@@ -83,7 +83,8 @@ XMLObject* SAML2ECPDecoder::decode(
const HTTPRequest* httpRequest = dynamic_cast<const HTTPRequest*>(&genericRequest);
if (httpRequest) {
string s = httpRequest->getContentType();
- if (s.find("application/vnd.paos+xml") == string::npos) {
+ if (s.find("application/vnd.paos+xml") == string::npos ||
+ s.find("application/x-www-form-urlencoded") != string::npos) {
log.warn("ignoring incorrect content type (%s)", s.c_str() ? s.c_str() : "none");
throw BindingException("Invalid content type for PAOS message.");
}
Index: opensaml-2.5.5/saml/saml2/binding/impl/SAML2POSTDecoder.cpp
===================================================================
--- opensaml-2.5.5.orig/saml/saml2/binding/impl/SAML2POSTDecoder.cpp
+++ opensaml-2.5.5/saml/saml2/binding/impl/SAML2POSTDecoder.cpp
@@ -89,11 +89,18 @@ XMLObject* SAML2POSTDecoder::decode(
throw BindingException("Unable to cast request object to HTTPRequest type.");
if (strcmp(httpRequest->getMethod(),"POST"))
throw BindingException("Invalid HTTP method ($1).", params(1, httpRequest->getMethod()));
- const char* msg = httpRequest->getParameter("SAMLResponse");
- if (!msg)
- msg = httpRequest->getParameter("SAMLRequest");
+
+ bool isRequest = false;
+ const char* msg = httpRequest->getParameter("SAMLRequest");
+ if (msg) {
+ isRequest = true;
+ } else {
+ msg = httpRequest->getParameter("SAMLResponse");
+ }
+
if (!msg)
throw BindingException("Request missing SAMLRequest or SAMLResponse form parameter.");
+
const char* state = httpRequest->getParameter("RelayState");
if (state)
relayState = state;
@@ -118,16 +125,20 @@ XMLObject* SAML2POSTDecoder::decode(
saml2::RootObject* root = nullptr;
StatusResponseType* response = nullptr;
- RequestAbstractType* request = dynamic_cast<RequestAbstractType*>(xmlObject.get());
- if (!request) {
+ RequestAbstractType* request = nullptr;
+ if (isRequest) {
+ request = dynamic_cast<RequestAbstractType*>(xmlObject.get());
+ if (!request) {
+ throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder was not a SAML 2.0 request message.");
+ }
+ root = static_cast<saml2::RootObject*>(request);
+ } else {
response = dynamic_cast<StatusResponseType*>(xmlObject.get());
- if (!response)
- throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message.");
+ if (!response) {
+ throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder was not a SAML 2.0 response message.");
+ }
root = static_cast<saml2::RootObject*>(response);
}
- else {
- root = static_cast<saml2::RootObject*>(request);
- }
SchemaValidators.validate(root);
Index: opensaml-2.5.5/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp
===================================================================
--- opensaml-2.5.5.orig/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp
+++ opensaml-2.5.5/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp
@@ -87,16 +87,24 @@ XMLObject* SAML2RedirectDecoder::decode(
const HTTPRequest* httpRequest=dynamic_cast<const HTTPRequest*>(&genericRequest);
if (!httpRequest)
throw BindingException("Unable to cast request object to HTTPRequest type.");
- const char* msg = httpRequest->getParameter("SAMLResponse");
- if (!msg)
- msg = httpRequest->getParameter("SAMLRequest");
+
+ bool isRequest = false;
+ const char* msg = httpRequest->getParameter("SAMLRequest");
+ if (msg) {
+ isRequest = true;
+ } else {
+ msg = httpRequest->getParameter("SAMLResponse");
+ }
+
if (!msg)
throw BindingException("Request missing SAMLRequest or SAMLResponse query string parameter.");
+
const char* state = httpRequest->getParameter("RelayState");
if (state)
relayState = state;
else
relayState.erase();
+
state = httpRequest->getParameter("SAMLEncoding");
if (state && strcmp(state,samlconstants::SAML20_BINDING_URL_ENCODING_DEFLATE)) {
log.warn("SAMLEncoding (%s) was not recognized", state);
@@ -136,16 +144,20 @@ XMLObject* SAML2RedirectDecoder::decode(
saml2::RootObject* root = nullptr;
StatusResponseType* response = nullptr;
- RequestAbstractType* request = dynamic_cast<RequestAbstractType*>(xmlObject.get());
- if (!request) {
+ RequestAbstractType* request = nullptr;
+ if (isRequest) {
+ request = dynamic_cast<RequestAbstractType*>(xmlObject.get());
+ if (!request) {
+ throw BindingException("XML content for SAML 2.0 HTTP-Redirect Decoder was not a SAML 2.0 request message.");
+ }
+ root = static_cast<saml2::RootObject*>(request);
+ } else {
response = dynamic_cast<StatusResponseType*>(xmlObject.get());
- if (!response)
- throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message.");
+ if (!response) {
+ throw BindingException("XML content for SAML 2.0 HTTP-Redirect Decoder was not a SAML 2.0 response message.");
+ }
root = static_cast<saml2::RootObject*>(response);
}
- else {
- root = static_cast<saml2::RootObject*>(request);
- }
SchemaValidators.validate(root);
Index: opensaml-2.5.5/saml/saml2/binding/impl/SAML2SOAPDecoder.cpp
===================================================================
--- opensaml-2.5.5.orig/saml/saml2/binding/impl/SAML2SOAPDecoder.cpp
+++ opensaml-2.5.5/saml/saml2/binding/impl/SAML2SOAPDecoder.cpp
@@ -84,7 +84,7 @@ XMLObject* SAML2SOAPDecoder::decode(
log.debug("validating input");
string s = genericRequest.getContentType();
- if (s.find("text/xml") == string::npos) {
+ if (s.find("text/xml") == string::npos || s.find("application/x-www-form-urlencoded") != string::npos) {
log.warn("ignoring incorrect content type (%s)", s.c_str() ? s.c_str() : "none");
throw BindingException("Invalid content type for SOAP message.");
}
