Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Fix on XPath searches + extra validations to avoid wrapping attacks #65

Merged
merged 4 commits into from
Nov 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions SECURITY-NOTICE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
Security vulnerability details for passport-wsfed-saml2 < 3.0.5
===============================================================

A vulnerability has been discovered in the passport-wsfed-saml2 library affecting versions < 3.0.5. This vulnerability allows an attacker to impersonate another user and potentially elevate their privileges if the SAML identity provider:

* signs SAML response and signs assertion
* does not sign SAML response and signs assertion

Developers using the passport-wsfed-saml2 Passport Strategy need to upgrade to the latest version: 3.0.5.

Updated packages are available on npm. To ensure delivery of additional bug fixes moving forward, please make sure your `package.json` file is updated to take patch and minor level updates of our libraries. See below:

```
{
"dependencies": {
"passport-wsfed-saml2": "^3.0.5"
}
}
```

## Upgrade Notes

This fix patches the library that your application runs, but will not impact your users, their current state, or any existing sessions.
4 changes: 2 additions & 2 deletions examples/login/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ passport.use(new wsfedsaml2(
homeRealm: '', // specify an identity provider to avoid showing the idp selector
identityProviderUrl: 'https://auth10-dev.accesscontrol.windows.net/v2/wsfederation',
// setup either a certificate base64 encoded (cer) or just the thumbprint of the certificate if public key is embedded in the signature

//cert: 'MIIDFjCCAf6gAwIBAgIQDRRprj9lv5RBvaQdlFltDzANBgkqhkiG9w0BAQUFADAvMS0wKwYDVQQDEyRhdXRoMTAtZGV2LmFjY2Vzc2NvbnRyb2wud2luZG93cy5uZXQwHhcNMTEwOTIxMDMzMjMyWhcNMTIwOTIwMDkzMjMyWjAvMS0wKwYDVQQDEyRhdXRoMTAtZGV2LmFjY2Vzc2NvbnRyb2wud2luZG93cy5uZXQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCEIAEB/KKT3ehNMy2MQEyJIQ14CnZ8DC2FZgL5Gw3UBSdRb9JinK/gw7yOQtwKfJUqeoZaUSAAdcdbgqwVxOnMBfWiYX7DGlEznSfqYVnjOWjqqjpoe0h6RaOkdWovDtoidmqVV1tWRJFjkj895clPxkLpnqqcycfXtSdZen0SroGyirD2mhMc9ccLbJ3zRnBNjlvpo5zow1zYows09tNC2EhGROL/OS4JNRQnJRICZC+WkA7Igf3xb4btJOzIPYhFiqCGrd/81CHmAyEuNzyc60I5yomDQfZ91Eb5Uk3F7mlfAlYB2aZwDwldLSOlVE8G1E5xFexF/5KyPC4ShNodAgMBAAGjLjAsMAsGA1UdDwQEAwIE8DAdBgNVHQ4EFgQUyYfx/r0czsPgTzitqey+fGMQpkcwDQYJKoZIhvcNAQEFBQADggEBAB5dgQlM3tKS+/cjlvMCPjZH0Iqo/Wxecri3YWi2iVziZ/TQ3dSV+J/iTyduN7rJmFQzTsNERcsgyAwblwnEKXXvlWo8G/+VDIMh3zVPNQFKns5WPkfkhoSVlnZPTQ8zdXAcWgDXbCgvdqIPozdgL+4l0W0XVL1ugA4/hmMXh4TyNd9Qj7MWvlmwVjevpSqN4wG735jAZFHb/L/vvc91uKqP+JvLNj8tPFVxatzi56X1V8jBM61Hx1Z9D0RCDjtmcQVysVEylW9O6mNy6ZrhLm0q5yecWudfBbTKDqRoCHQRjrMU2c5q/ZFDtgjLim7FaNxFbgTyjeRCPclEhfemYVg='
thumbprints: ['a3cff17cbf7e793a97861390eb698d00e9598537']
},
Expand Down Expand Up @@ -91,7 +91,7 @@ app.use(bodyParser.urlencoded({
extended: true
}));
app.use(methodOverride());
app.use(session({
app.use(session({
secret: 'keyboard cat',
resave: false,
saveUninitialized: true
Expand Down
54 changes: 29 additions & 25 deletions lib/passport-wsfed-saml2/saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,13 @@ SAML.prototype.validateSignature = function (xml, options, callback) {
xml = utils.parseSamlResponse(xml);

var signaturePath = this.options.signaturePath || options.signaturePath;
var signature = xpath.select(signaturePath, xml)[0];
if (!signature)
var signatures = xpath.select(signaturePath, xml);
if (signatures.length === 0) {
return callback(new Error('Signature is missing (xpath: ' + signaturePath + ')'));
} else if (signatures.length > 1) {
return callback(new Error('Signature was found more than one time (xpath: ' + signaturePath + ')'));
}
var signature = signatures[0];

var sig = new xmlCrypto.SignedXml(null, { idAttribute: 'AssertionID' });
sig.keyInfoProvider = {
Expand Down Expand Up @@ -112,7 +116,7 @@ SAML.prototype.validateSignature = function (xml, options, callback) {
};

SAML.prototype.validateExpiration = function (samlAssertion, version) {
var conditions = xpath.select("//*[local-name(.)='Conditions']", samlAssertion);
var conditions = xpath.select(".//*[local-name(.)='Conditions']", samlAssertion);
if (!conditions || conditions.length === 0) return true;

var notBefore = new Date(conditions[0].getAttribute('NotBefore'));
Expand All @@ -131,9 +135,9 @@ SAML.prototype.validateExpiration = function (samlAssertion, version) {
SAML.prototype.validateAudience = function (samlAssertion, realm, version) {
var audience;
if (version === '2.0') {
audience = xpath.select("//*[local-name(.)='Conditions']/*[local-name(.)='AudienceRestriction']/*[local-name(.)='Audience']", samlAssertion);
audience = xpath.select(".//*[local-name(.)='Conditions']/*[local-name(.)='AudienceRestriction']/*[local-name(.)='Audience']", samlAssertion);
} else {
audience = xpath.select("//*[local-name(.)='Conditions']/*[local-name(.)='AudienceRestrictionCondition']/*[local-name(.)='Audience']", samlAssertion);
audience = xpath.select(".//*[local-name(.)='Conditions']/*[local-name(.)='AudienceRestrictionCondition']/*[local-name(.)='Audience']", samlAssertion);
}

if (!audience || audience.length === 0) return false;
Expand All @@ -152,7 +156,7 @@ SAML.prototype.validateNameQualifier = function (samlAssertion, issuer) {
// Ignore validation if the format is not persistent or transient
return true;
}

return nameID.NameQualifier === issuer
};

Expand All @@ -168,8 +172,8 @@ SAML.prototype.validateSPNameQualifier = function (samlAssertion, audience) {
// Ignore validation if the format is not persistent or transient
return true;
}
return nameID.SPNameQualifier === audience

return nameID.SPNameQualifier === audience
};

// https://www.oasis-open.org/committees/download.php/35711/sstc-saml-core-errata-2.0-wd-06-diff.pdf
Expand All @@ -179,17 +183,17 @@ SAML.prototype.validateSPNameQualifier = function (samlAssertion, audience) {
// example, this attribute might indicate that the assertion must be delivered to a particular network
// endpoint in order to prevent an intermediary from redirecting it someplace else.
SAML.prototype.validateRecipient = function(samlAssertion, recipientUrl){
var subjectConfirmationData = xpath.select("//*[local-name(.)='Subject']/*[local-name(.)='SubjectConfirmation']/*[local-name(.)='SubjectConfirmationData']", samlAssertion);
var subjectConfirmationData = xpath.select(".//*[local-name(.)='Subject']/*[local-name(.)='SubjectConfirmation']/*[local-name(.)='SubjectConfirmationData']", samlAssertion);

// subjectConfirmationData is optional in the spec. Only validate if the assertion contains a recipient
if (!subjectConfirmationData || subjectConfirmationData.length === 0){
if (!subjectConfirmationData || subjectConfirmationData.length === 0){
return true;
}

var recipient = subjectConfirmationData[0].getAttribute('Recipient');

var valid = !recipient || recipient === recipientUrl;

if (!valid){
this.eventEmitter.emit('recipientValidationFailed', {
configuredRecipient: recipientUrl,
Expand All @@ -202,26 +206,26 @@ SAML.prototype.validateRecipient = function(samlAssertion, recipientUrl){

SAML.prototype.parseAttributes = function (samlAssertion, version) {
function getAttributes(samlAssertion) {
var attributes = xpath.select("//*[local-name(.)='AttributeStatement']/*[local-name(.)='Attribute']", samlAssertion);
var attributes = xpath.select(".//*[local-name(.)='AttributeStatement']/*[local-name(.)='Attribute']", samlAssertion);
return attributes;
}

function getSessionIndex(samlAssertion) {
var authnStatement = xpath.select("//*[local-name(.)='AuthnStatement']", samlAssertion);
var authnStatement = xpath.select(".//*[local-name(.)='AuthnStatement']", samlAssertion);
var sessionIndex = authnStatement.length > 0 && authnStatement[0].attributes.length > 0 ?
authnStatement[0].getAttribute('SessionIndex') : undefined;
return sessionIndex || undefined;
}

function getNameID11(samlAssertion) {
var nameId = xpath.select("//*[local-name(.)='AuthenticationStatement']/*[local-name(.)='Subject']/*[local-name(.)='NameIdentifier']", samlAssertion);
var nameId = xpath.select(".//*[local-name(.)='AuthenticationStatement']/*[local-name(.)='Subject']/*[local-name(.)='NameIdentifier']", samlAssertion);

if (nameId.length === 0) {
// only for backward compatibility with adfs
nameId = xpath.select("//*[local-name(.)='AttributeStatement']/*[local-name(.)='Subject']/*[local-name(.)='NameIdentifier']", samlAssertion);
nameId = xpath.select(".//*[local-name(.)='AttributeStatement']/*[local-name(.)='Subject']/*[local-name(.)='NameIdentifier']", samlAssertion);
if (nameId.length === 0) return;
}

return nameId[0].textContent;
}

Expand All @@ -239,7 +243,7 @@ SAML.prototype.parseAttributes = function (samlAssertion, version) {
}

function getAuthContext20(samlAssertion) {
var authnContext = xpath.select("//*[local-name(.)='AuthnStatement']/*[local-name(.)='AuthnContext']/*[local-name(.)='AuthnContextClassRef']", samlAssertion);
var authnContext = xpath.select(".//*[local-name(.)='AuthnStatement']/*[local-name(.)='AuthnContext']/*[local-name(.)='AuthnContextClassRef']", samlAssertion);
if (authnContext.length === 0) return;
return authnContext[0].textContent;
}
Expand Down Expand Up @@ -296,7 +300,7 @@ SAML.prototype.validateSamlAssertion = function (samlAssertion, callback) {
self.validateSignature(samlAssertion.toString(), {
cert: self.options.cert,
thumbprints: self.options.thumbprints,
signaturePath: "//*[local-name(.)='Assertion']/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']" }, function(err) {
signaturePath: ".//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']" }, function(err) {
if (err) return callback(err);

self.parseAssertion(samlAssertion, callback);
Expand All @@ -308,7 +312,7 @@ SAML.prototype.parseAssertion = function(samlAssertion, callback) {
if (self.options.extractSAMLAssertion){
samlAssertion = self.options.extractSAMLAssertion(samlAssertion);
}

samlAssertion = utils.parseSamlResponse(samlAssertion);

if (!samlAssertion.getAttribute)
Expand Down Expand Up @@ -341,13 +345,13 @@ SAML.prototype.parseAssertion = function(samlAssertion, callback) {

var issuer;
if (version === '2.0') {
var issuerNode = xpath.select("//*[local-name(.)='Issuer']", samlAssertion);
var issuerNode = xpath.select(".//*[local-name(.)='Issuer']", samlAssertion);
if (issuerNode.length > 0) issuer = issuerNode[0].textContent;
} else {
issuer = samlAssertion.getAttribute('Issuer');
}


this.eventEmitter.emit('parseAssertion', {
issuer: issuer,
version: version,
Expand All @@ -363,7 +367,7 @@ SAML.prototype.parseAssertion = function(samlAssertion, callback) {
// Validate the SP name qualifier in the NameID element if found with the issuer
if (self.options.checkSPNameQualifier && !self.validateSPNameQualifier(samlAssertion, self.options.realm)){
return callback(new Error('SPNameQualifier attribute in the NameID element does not match ' + self.options.realm), null);
}
}

if (!profile.email && profile['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress']) {
profile.email = profile['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress'];
Expand All @@ -373,7 +377,7 @@ SAML.prototype.parseAssertion = function(samlAssertion, callback) {
};

function getNameID20(samlAssertion) {
var nameId = xpath.select("//*[local-name(.)='Subject']/*[local-name(.)='NameID']", samlAssertion);
var nameId = xpath.select(".//*[local-name(.)='Subject']/*[local-name(.)='NameID']", samlAssertion);
if (nameId.length === 0) return;
var element = nameId[0];
var result = {
Expand Down
33 changes: 23 additions & 10 deletions lib/passport-wsfed-saml2/samlp.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,17 +248,21 @@ Samlp.prototype = {
callback(null, assertion);
};

var assertions = samlpResponse.getElementsByTagNameNS(saml2Namespace, 'Assertion');
if (assertions.length > 1) {
return done(new Error('A SAMLResponse can contains only one Assertion element.'));
var foundAssertions = xpath.select("//*[local-name(.)='Assertion']", samlpResponse);
if (foundAssertions.length > 1) {
return done(new Error('A SAMLResponse can contain only one Assertion element.'));
}

// After being sure no more "Assertion" elements are found, we extract it from the expected place
var assertions = xpath.select("/*[local-name(.)='Response']/*[local-name(.)='Assertion' and namespace-uri(.)='" + saml2Namespace + "']", samlpResponse);
var token = assertions[0];

if (!token) {
// check for encrypted assertion
var encryptedAssertion = samlpResponse.getElementsByTagNameNS(saml2Namespace, 'EncryptedAssertion');
var encryptedAssertionPath = "/*[local-name(.)='Response']/*[local-name(.)='EncryptedAssertion' and namespace-uri(.)='" + saml2Namespace + "']";
var encryptedAssertion = xpath.select(encryptedAssertionPath, samlpResponse);
if (encryptedAssertion.length > 1) {
return done(new Error('A SAMLResponse can contains only one EncryptedAssertion element.'));
return done(new Error('A SAMLResponse can contain only one EncryptedAssertion element.'));
}

var encryptedToken = encryptedAssertion[0];
Expand Down Expand Up @@ -316,10 +320,14 @@ Samlp.prototype = {
samlResponse = utils.parseSamlResponse(samlResponse);

// Check that the saml Resopnse actually has a Response object
var responseXML = xpath.select("//*[local-name(.)='Response']", samlResponse)[0];
if (!responseXML){
var responseXMLs = xpath.select("//*[local-name(.)='Response']", samlResponse);
if (responseXMLs.length === 0) {
return callback(new Error('XML is not a valid saml response'));
}
if (responseXMLs.length > 1) {
return callback(new Error('SAMLResponse should be unique'));
}
var responseXML = responseXMLs[0];

self.isValidResponseID(responseXML.getAttribute('ID'), function(err){
if (err && self.options.checkResponseID) { return callback(err); }
Expand Down Expand Up @@ -349,7 +357,7 @@ Samlp.prototype = {
return callback(new Error('Destination endpoint ' + destination + ' did not match ' + self.options.destinationUrl));
}
}

// check status
var samlStatus = self.getSamlStatus(samlResponse);

Expand All @@ -369,11 +377,16 @@ Samlp.prototype = {
return callback(new Error('saml response does not contain an Assertion element (Status: ' + samlStatus.code + ')'));
}

var samlResponseSignaturePath = "//*[local-name(.)='Response']/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']";
var samlResponseSignaturePath = "/*[local-name(.)='Response']/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']";
var isResponseSigned = xpath.select(samlResponseSignaturePath, samlResponse).length > 0;
var samlAssertionSignaturePath = "//*[local-name(.)='Assertion']/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']";
var samlAssertionSignaturePath = ".//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']";
var isAssertionSigned = xpath.select(samlAssertionSignaturePath, assertion).length > 0;

self.eventEmitter.emit('SAMLResponse:signatures', {
isResponseSigned: isResponseSigned,
isAssertionSigned: isAssertionSigned
});

if (!isResponseSigned && !isAssertionSigned) {
return callback(new Error('neither the response nor the assertion are signed'));
}
Expand Down
4 changes: 2 additions & 2 deletions lib/passport-wsfed-saml2/strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ WsFedSaml2Strategy.prototype._authenticate_saml = function (req, state) {

self._wsfed.retrieveToken(req, function(err, token) {
if (err) return self.fail(err, err.status || 400);

self.options.recipientUrl = self.options.recipientUrl || getReqUrl(req);

self._saml.validateSamlAssertion(token, function (err, profile) {
Expand Down Expand Up @@ -214,7 +214,7 @@ WsFedSaml2Strategy.prototype.authenticate = function (req, opts) {
}

var samlResponseDom = utils.parseSamlResponse(samlResponse);

// If options are not set, we set the expected value from the request object
var req_full_url = getReqUrl(req);
self.options.destinationUrl = self.options.destinationUrl || req_full_url;
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "passport-wsfed-saml2",
"version": "3.0.4",
"version": "3.0.5",
"description": "SAML2 Protocol and WS-Fed library",
"scripts": {
"test": "mocha --reporter spec --recursive"
Expand Down
Loading