Skip to content

Commit 874b410

Browse files
mauromolmauromol
committed
Properly escape text to produce valid XML
The generation methods for metadata, AuthnRequest, LogoutRequest and LogoutResponse XML documents and messages have been changed so that any text that could be specified by the user is properly escaped in order to produce a valid XML output: characters like &, ", < and > are then replaced with the corresponding XML entities (&amp; &quot;, &lt;, &gt;). The commons-lang StringEscapeUtils class has been used, although being deprecated: the deprecation warning suggests to switch to an equivalent class of commons-text, which java-saml currently does not depend on. I leave the decision to possibly switch to it for future evaluation: the escaping logic was indeed implemented in the Util class, so any implementation change of it just needs to change Util.toXml(String) method. Closes #309 and #305.
1 parent 523786b commit 874b410

File tree

11 files changed

+657
-49
lines changed

11 files changed

+657
-49
lines changed

‎core/src/main/java/com/onelogin/saml2/authn/AuthnRequest.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -186,15 +186,15 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
186186
String destinationStr = "";
187187
URL sso = settings.getIdpSingleSignOnServiceUrl();
188188
if (sso != null) {
189-
destinationStr = " Destination=\"" + sso.toString() + "\"";
189+
destinationStr = " Destination=\"" + Util.toXml(sso.toString()) + "\"";
190190
}
191191
valueMap.put("destinationStr", destinationStr);
192192

193193
String subjectStr = "";
194194
if (nameIdValueReq != null && !nameIdValueReq.isEmpty()) {
195195
String nameIDFormat = settings.getSpNameIDFormat();
196196
subjectStr = "<saml:Subject>";
197-
subjectStr += "<saml:NameID Format=\"" + nameIDFormat + "\">" + nameIdValueReq + "</saml:NameID>";
197+
subjectStr += "<saml:NameID Format=\"" + Util.toXml(nameIDFormat) + "\">" + Util.toXml(nameIdValueReq) + "</saml:NameID>";
198198
subjectStr += "<saml:SubjectConfirmation Method=\"urn:oasis:names:tc:SAML:2.0:cm:bearer\"></saml:SubjectConfirmation>";
199199
subjectStr += "</saml:Subject>";
200200
}
@@ -206,7 +206,7 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
206206
if (settings.getWantNameIdEncrypted()) {
207207
nameIDPolicyFormat = Constants.NAMEID_ENCRYPTED;
208208
}
209-
nameIDPolicyStr = "<samlp:NameIDPolicy Format=\"" + nameIDPolicyFormat + "\" AllowCreate=\"true\" />";
209+
nameIDPolicyStr = "<samlp:NameIDPolicy Format=\"" + Util.toXml(nameIDPolicyFormat) + "\" AllowCreate=\"true\" />";
210210
}
211211
valueMap.put("nameIDPolicyStr", nameIDPolicyStr);
212212

@@ -215,25 +215,25 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
215215
if (organization != null) {
216216
String displayName = organization.getOrgDisplayName();
217217
if (!displayName.isEmpty()) {
218-
providerStr = " ProviderName=\""+ displayName + "\"";
218+
providerStr = " ProviderName=\""+ Util.toXml(displayName) + "\"";
219219
}
220220
}
221221
valueMap.put("providerStr", providerStr);
222222

223223
String issueInstantString = Util.formatDateTime(issueInstant.getTimeInMillis());
224224
valueMap.put("issueInstant", issueInstantString);
225-
valueMap.put("id", String.valueOf(id));
226-
valueMap.put("assertionConsumerServiceURL", String.valueOf(settings.getSpAssertionConsumerServiceUrl()));
227-
valueMap.put("protocolBinding", settings.getSpAssertionConsumerServiceBinding());
228-
valueMap.put("spEntityid", settings.getSpEntityId());
225+
valueMap.put("id", Util.toXml(String.valueOf(id)));
226+
valueMap.put("assertionConsumerServiceURL", Util.toXml(String.valueOf(settings.getSpAssertionConsumerServiceUrl())));
227+
valueMap.put("protocolBinding", Util.toXml(settings.getSpAssertionConsumerServiceBinding()));
228+
valueMap.put("spEntityid", Util.toXml(settings.getSpEntityId()));
229229

230230
String requestedAuthnContextStr = "";
231231
List<String> requestedAuthnContexts = settings.getRequestedAuthnContext();
232232
if (requestedAuthnContexts != null && !requestedAuthnContexts.isEmpty()) {
233233
String requestedAuthnContextCmp = settings.getRequestedAuthnContextComparison();
234-
requestedAuthnContextStr = "<samlp:RequestedAuthnContext Comparison=\"" + requestedAuthnContextCmp + "\">";
234+
requestedAuthnContextStr = "<samlp:RequestedAuthnContext Comparison=\"" + Util.toXml(requestedAuthnContextCmp) + "\">";
235235
for (String requestedAuthnContext : requestedAuthnContexts) {
236-
requestedAuthnContextStr += "<saml:AuthnContextClassRef>" + requestedAuthnContext + "</saml:AuthnContextClassRef>";
236+
requestedAuthnContextStr += "<saml:AuthnContextClassRef>" + Util.toXml(requestedAuthnContext) + "</saml:AuthnContextClassRef>";
237237
}
238238
requestedAuthnContextStr += "</samlp:RequestedAuthnContext>";
239239
}

‎core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,19 +282,19 @@ public String getLogoutRequestXml() {
282282
private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
283283
Map<String, String> valueMap = new HashMap<String, String>();
284284

285-
valueMap.put("id", id);
285+
valueMap.put("id", Util.toXml(id));
286286

287287
String issueInstantString = Util.formatDateTime(issueInstant.getTimeInMillis());
288288
valueMap.put("issueInstant", issueInstantString);
289289

290290
String destinationStr = "";
291291
URL slo = settings.getIdpSingleLogoutServiceUrl();
292292
if (slo != null) {
293-
destinationStr = " Destination=\"" + slo.toString() + "\"";
293+
destinationStr = " Destination=\"" + Util.toXml(slo.toString()) + "\"";
294294
}
295295
valueMap.put("destinationStr", destinationStr);
296296

297-
valueMap.put("issuer", settings.getSpEntityId());
297+
valueMap.put("issuer", Util.toXml(settings.getSpEntityId()));
298298

299299
String nameIdFormat = null;
300300
String spNameQualifier = this.nameIdSPNameQualifier;
@@ -337,7 +337,7 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
337337

338338
String sessionIndexStr = "";
339339
if (sessionIndex != null) {
340-
sessionIndexStr = " <samlp:SessionIndex>" + sessionIndex + "</samlp:SessionIndex>";
340+
sessionIndexStr = " <samlp:SessionIndex>" + Util.toXml(sessionIndex) + "</samlp:SessionIndex>";
341341
}
342342
valueMap.put("sessionIndexStr", sessionIndexStr);
343343

‎core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,31 +397,31 @@ public void build() {
397397
private StrSubstitutor generateSubstitutor(Saml2Settings settings, String statusCode) {
398398
Map<String, String> valueMap = new HashMap<String, String>();
399399

400-
valueMap.put("id", id);
400+
valueMap.put("id", Util.toXml(id));
401401

402402
String issueInstantString = Util.formatDateTime(issueInstant.getTimeInMillis());
403403
valueMap.put("issueInstant", issueInstantString);
404404

405405
String destinationStr = "";
406406
URL slo = settings.getIdpSingleLogoutServiceResponseUrl();
407407
if (slo != null) {
408-
destinationStr = " Destination=\"" + slo.toString() + "\"";
408+
destinationStr = " Destination=\"" + Util.toXml(slo.toString()) + "\"";
409409
}
410410
valueMap.put("destinationStr", destinationStr);
411411

412412
String inResponseStr = "";
413413
if (inResponseTo != null) {
414-
inResponseStr = " InResponseTo=\"" + inResponseTo + "\"";
414+
inResponseStr = " InResponseTo=\"" + Util.toXml(inResponseTo) + "\"";
415415
}
416416
valueMap.put("inResponseStr", inResponseStr);
417417

418418
String statusStr = "";
419419
if (statusCode != null) {
420-
statusStr = "Value=\"" + statusCode + "\"";
420+
statusStr = "Value=\"" + Util.toXml(statusCode) + "\"";
421421
}
422422
valueMap.put("statusStr", statusStr);
423423

424-
valueMap.put("issuer", settings.getSpEntityId());
424+
valueMap.put("issuer", Util.toXml(settings.getSpEntityId()));
425425

426426
return new StrSubstitutor(valueMap);
427427
}

‎core/src/main/java/com/onelogin/saml2/settings/Metadata.java‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.onelogin.saml2.settings;
22

3+
import static com.onelogin.saml2.util.Util.toXml;
4+
35
import java.net.URL;
46
import java.util.Arrays;
57
import java.util.Calendar;
@@ -126,7 +128,7 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) throws Certif
126128
Map<String, String> valueMap = new HashMap<String, String>();
127129
Boolean wantsEncrypted = settings.getWantAssertionsEncrypted() || settings.getWantNameIdEncrypted();
128130

129-
valueMap.put("id", Util.generateUniqueID(settings.getUniqueIDPrefix()));
131+
valueMap.put("id", Util.toXml(Util.generateUniqueID(settings.getUniqueIDPrefix())));
130132
String validUntilTimeStr = "";
131133
if (validUntilTime != null) {
132134
String validUntilTimeValue = Util.formatDateTime(validUntilTime.getTimeInMillis());
@@ -141,12 +143,12 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) throws Certif
141143
}
142144
valueMap.put("cacheDurationStr", cacheDurationStr);
143145

144-
valueMap.put("spEntityId", settings.getSpEntityId());
146+
valueMap.put("spEntityId", Util.toXml(settings.getSpEntityId()));
145147
valueMap.put("strAuthnsign", String.valueOf(settings.getAuthnRequestsSigned()));
146148
valueMap.put("strWsign", String.valueOf(settings.getWantAssertionsSigned()));
147-
valueMap.put("spNameIDFormat", settings.getSpNameIDFormat());
148-
valueMap.put("spAssertionConsumerServiceBinding", settings.getSpAssertionConsumerServiceBinding());
149-
valueMap.put("spAssertionConsumerServiceUrl", settings.getSpAssertionConsumerServiceUrl().toString());
149+
valueMap.put("spNameIDFormat", Util.toXml(settings.getSpNameIDFormat()));
150+
valueMap.put("spAssertionConsumerServiceBinding", Util.toXml(settings.getSpAssertionConsumerServiceBinding()));
151+
valueMap.put("spAssertionConsumerServiceUrl", Util.toXml(settings.getSpAssertionConsumerServiceUrl().toString()));
150152
valueMap.put("sls", toSLSXml(settings.getSpSingleLogoutServiceUrl(), settings.getSpSingleLogoutServiceBinding()));
151153

152154
valueMap.put("strAttributeConsumingService", getAttributeConsumingServiceXml());
@@ -198,10 +200,10 @@ private String getAttributeConsumingServiceXml() {
198200

199201
attributeConsumingServiceXML.append("<md:AttributeConsumingService index=\"1\">");
200202
if (serviceName != null && !serviceName.isEmpty()) {
201-
attributeConsumingServiceXML.append("<md:ServiceName xml:lang=\"en\">" + serviceName + "</md:ServiceName>");
203+
attributeConsumingServiceXML.append("<md:ServiceName xml:lang=\"en\">" + Util.toXml(serviceName) + "</md:ServiceName>");
202204
}
203205
if (serviceDescription != null && !serviceDescription.isEmpty()) {
204-
attributeConsumingServiceXML.append("<md:ServiceDescription xml:lang=\"en\">" + serviceDescription + "</md:ServiceDescription>");
206+
attributeConsumingServiceXML.append("<md:ServiceDescription xml:lang=\"en\">" + Util.toXml(serviceDescription) + "</md:ServiceDescription>");
205207
}
206208
if (requestedAttributes != null && !requestedAttributes.isEmpty()) {
207209
for (RequestedAttribute requestedAttribute : requestedAttributes) {
@@ -214,15 +216,15 @@ private String getAttributeConsumingServiceXml() {
214216
String contentStr = "<md:RequestedAttribute";
215217

216218
if (name != null && !name.isEmpty()) {
217-
contentStr += " Name=\"" + name + "\"";
219+
contentStr += " Name=\"" + Util.toXml(name) + "\"";
218220
}
219221

220222
if (nameFormat != null && !nameFormat.isEmpty()) {
221-
contentStr += " NameFormat=\"" + nameFormat + "\"";
223+
contentStr += " NameFormat=\"" + Util.toXml(nameFormat) + "\"";
222224
}
223225

224226
if (friendlyName != null && !friendlyName.isEmpty()) {
225-
contentStr += " FriendlyName=\"" + friendlyName + "\"";
227+
contentStr += " FriendlyName=\"" + Util.toXml(friendlyName) + "\"";
226228
}
227229

228230
if (isRequired != null) {
@@ -232,7 +234,7 @@ private String getAttributeConsumingServiceXml() {
232234
if (attrValues != null && !attrValues.isEmpty()) {
233235
contentStr += ">";
234236
for (String attrValue : attrValues) {
235-
contentStr += "<saml:AttributeValue xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\">" + attrValue + "</saml:AttributeValue>";
237+
contentStr += "<saml:AttributeValue xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\">" + Util.toXml(attrValue) + "</saml:AttributeValue>";
236238
}
237239
attributeConsumingServiceXML.append(contentStr + "</md:RequestedAttribute>");
238240
} else {
@@ -256,9 +258,9 @@ private String toContactsXml(List<Contact> contacts) {
256258
StringBuilder contactsXml = new StringBuilder();
257259

258260
for (Contact contact : contacts) {
259-
contactsXml.append("<md:ContactPerson contactType=\"" + contact.getContactType() + "\">");
260-
contactsXml.append("<md:GivenName>" + contact.getGivenName() + "</md:GivenName>");
261-
contactsXml.append("<md:EmailAddress>" + contact.getEmailAddress() + "</md:EmailAddress>");
261+
contactsXml.append("<md:ContactPerson contactType=\"" + Util.toXml(contact.getContactType()) + "\">");
262+
contactsXml.append("<md:GivenName>" + Util.toXml(contact.getGivenName()) + "</md:GivenName>");
263+
contactsXml.append("<md:EmailAddress>" + Util.toXml(contact.getEmailAddress()) + "</md:EmailAddress>");
262264
contactsXml.append("</md:ContactPerson>");
263265
}
264266

@@ -276,10 +278,10 @@ private String toOrganizationXml(Organization organization) {
276278

277279
if (organization != null) {
278280
String lang = organization.getOrgLangAttribute();
279-
orgXml = "<md:Organization><md:OrganizationName xml:lang=\"" + lang + "\">" + organization.getOrgName()
280-
+ "</md:OrganizationName><md:OrganizationDisplayName xml:lang=\"" + lang + "\">"
281-
+ organization.getOrgDisplayName() + "</md:OrganizationDisplayName><md:OrganizationURL xml:lang=\""
282-
+ lang + "\">" + organization.getOrgUrl() + "</md:OrganizationURL></md:Organization>";
281+
orgXml = "<md:Organization><md:OrganizationName xml:lang=\"" + Util.toXml(lang) + "\">" + Util.toXml(organization.getOrgName())
282+
+ "</md:OrganizationName><md:OrganizationDisplayName xml:lang=\"" + Util.toXml(lang) + "\">"
283+
+ Util.toXml(organization.getOrgDisplayName()) + "</md:OrganizationDisplayName><md:OrganizationURL xml:lang=\""
284+
+ Util.toXml(lang) + "\">" + Util.toXml(organization.getOrgUrl()) + "</md:OrganizationURL></md:Organization>";
283285
}
284286
return orgXml;
285287
}
@@ -316,7 +318,7 @@ private String toX509KeyDescriptorsXML(X509Certificate certCurrent, X509Certific
316318
keyDescriptorXml.append("<md:KeyDescriptor use=\"signing\">");
317319
keyDescriptorXml.append("<ds:KeyInfo xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\">");
318320
keyDescriptorXml.append("<ds:X509Data>");
319-
keyDescriptorXml.append("<ds:X509Certificate>"+certString+"</ds:X509Certificate>");
321+
keyDescriptorXml.append("<ds:X509Certificate>"+Util.toXml(certString)+"</ds:X509Certificate>");
320322
keyDescriptorXml.append("</ds:X509Data>");
321323
keyDescriptorXml.append("</ds:KeyInfo>");
322324
keyDescriptorXml.append("</md:KeyDescriptor>");
@@ -325,7 +327,7 @@ private String toX509KeyDescriptorsXML(X509Certificate certCurrent, X509Certific
325327
keyDescriptorXml.append("<md:KeyDescriptor use=\"encryption\">");
326328
keyDescriptorXml.append("<ds:KeyInfo xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\">");
327329
keyDescriptorXml.append("<ds:X509Data>");
328-
keyDescriptorXml.append("<ds:X509Certificate>"+certString+"</ds:X509Certificate>");
330+
keyDescriptorXml.append("<ds:X509Certificate>"+Util.toXml(certString)+"</ds:X509Certificate>");
329331
keyDescriptorXml.append("</ds:X509Data>");
330332
keyDescriptorXml.append("</ds:KeyInfo>");
331333
keyDescriptorXml.append("</md:KeyDescriptor>");
@@ -343,8 +345,8 @@ private String toSLSXml(URL spSingleLogoutServiceUrl, String spSingleLogoutServi
343345
StringBuilder slsXml = new StringBuilder();
344346

345347
if (spSingleLogoutServiceUrl != null) {
346-
slsXml.append("<md:SingleLogoutService Binding=\"" + spSingleLogoutServiceBinding + "\"");
347-
slsXml.append(" Location=\"" + spSingleLogoutServiceUrl.toString() + "\"/>");
348+
slsXml.append("<md:SingleLogoutService Binding=\"" + Util.toXml(spSingleLogoutServiceBinding) + "\"");
349+
slsXml.append(" Location=\"" + Util.toXml(spSingleLogoutServiceUrl.toString()) + "\"/>");
348350
}
349351
return slsXml.toString();
350352
}

‎core/src/main/java/com/onelogin/saml2/util/Util.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@
6363
import javax.xml.xpath.XPathFactory;
6464
import javax.xml.xpath.XPathFactoryConfigurationException;
6565

66-
import com.onelogin.saml2.model.hsm.HSM;
6766
import org.apache.commons.codec.binary.Base64;
6867
import org.apache.commons.codec.digest.DigestUtils;
68+
import org.apache.commons.lang3.StringEscapeUtils;
6969
import org.apache.commons.lang3.StringUtils;
7070
import org.apache.xml.security.encryption.EncryptedData;
7171
import org.apache.xml.security.encryption.EncryptedKey;
@@ -95,6 +95,7 @@
9595
import com.onelogin.saml2.exception.ValidationError;
9696
import com.onelogin.saml2.exception.XMLEntityException;
9797
import com.onelogin.saml2.model.SamlResponseStatus;
98+
import com.onelogin.saml2.model.hsm.HSM;
9899

99100

100101
/**
@@ -445,7 +446,7 @@ public static String convertDocumentToString(Document doc, Boolean c14n) {
445446
* @return the Document object
446447
*/
447448
public static String convertDocumentToString(Document doc) {
448-
return convertDocumentToString(doc, false);
449+
return convertDocumentToString(doc, false);
449450
}
450451

451452
/**
@@ -1984,6 +1985,17 @@ public static DateTime parseDateTime(String dateTime) {
19841985
}
19851986
return parsedData;
19861987
}
1988+
1989+
/**
1990+
* Escape a text so that it can be safely used within an XML element contents or attribute value.
1991+
*
1992+
* @param text
1993+
* the text to escape
1994+
* @return the escaped text (<code>null</code> if the input is <code>null</code>)
1995+
*/
1996+
public static String toXml(String text) {
1997+
return StringEscapeUtils.escapeXml10(text);
1998+
}
19871999

19882000
private static String toStringUtf8(byte[] bytes) {
19892001
try {

0 commit comments

Comments
 (0)