Skip to content

Commit

Permalink
Modify rule S5679: Change text to education framework format (APPSEC-…
Browse files Browse the repository at this point in the history
…1107) (#3159)

## Review

A dedicated reviewer checked the rule description successfully for:

- [ ] logical errors and incorrect information
- [ ] information gaps and missing content
- [ ] text style and tone
- [ ] PR summary and labels follow [the
guidelines](https://github.com/SonarSource/rspec/#to-modify-an-existing-rule)

---------

Co-authored-by: gaetan-ferry-sonarsource <[email protected]>
  • Loading branch information
1 parent 624fbe3 commit 38c07d1
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 86 deletions.
53 changes: 1 addition & 52 deletions rules/S5679/java/metadata.json
Original file line number Diff line number Diff line change
@@ -1,52 +1 @@
{
"title": "OpenSAML2 should be configured to prevent authentication bypass",
"type": "VULNERABILITY",
"code": {
"impacts": {
"SECURITY": "MEDIUM"
},
"attribute": "COMPLETE"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "10min"
},
"tags": [
"spring"
],
"extra": {
"replacementRules": [

],
"legacyKeys": [

]
},
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-5679",
"sqKey": "S5679",
"scope": "Main",
"securityStandards": {
"OWASP": [
"A9",
"A2"
],
"OWASP Top 10 2021": [
"A6",
"A7"
],
"PCI DSS 3.2": [
"6.2",
"6.5.10"
],
"PCI DSS 4.0": [
"6.3.3",
"6.2.4"
]
},
"defaultQualityProfiles": [
"Sonar way"
],
"quickfix": "unknown"
}
{ }
92 changes: 58 additions & 34 deletions rules/S5679/java/rule.adoc
Original file line number Diff line number Diff line change
@@ -1,82 +1,106 @@
The Security Assertion Markup Language (SAML) is a widely used standard in single sign-on systems. In a simplified version, the user authenticates to an Identity Provider which generates a signed SAML Response. This response is then forwarded to a Service Provider for validation and authentication.

== Why is this an issue?

In 2018, Duo Security found a new vulnerability class that affects SAML-based single sign-on (SSO) systems and this led to the following vulnerabilities being disclosed: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11427[CVE-2017-11427], https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11428[CVE-2017-11428], https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11429[CVE-2017-11429], https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11430[CVE-2017-11430], https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-0489[CVE-2018-0489], https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-7340[CVE-2018-7340].
If the Service Provider does not manage to properly validate the incoming SAML response message signatures, attackers might be able to manipulate the response content without the application noticing. Especially, they might be able to alter the authentication-targeted user.

=== What is the potential impact?

From a specially crafted ``++<SAMLResponse>++`` file, an attacker having already access to the SAML system with his own account can bypass the authentication mechanism and be authenticated as another user.
By exploiting this vulnerability, an attacker can manipulate the SAML Response to impersonate a different user. This, in turn, can have various consequences on the application's security.

This is due to the fact that SAML protocol rely on XML format and how the underlying XML parser interprets XML comments.
=== Unauthorized Access

Exploiting this vulnerability allows an attacker with authenticated access to impersonate other users within the SAML-based SSO system. This can lead to unauthorized access to sensitive information, resources, or functionalities the attacker should not have. By masquerading as legitimate users, the attacker can bypass authentication mechanisms and gain unauthorized privileges, potentially compromising the entire system. By impersonating a user with higher privileges, the attacker can gain access to additional resources. Privilege escalation can lead to further compromise of other systems and unauthorized access to critical infrastructure.

If an attacker manage to change the ``++<NameID>++`` field identifying the authenticated user with XML comments, he can exploit the vulnerability.
=== Data Breaches

With the ability to impersonate other users, an attacker can gain access to sensitive data stored within the SAML-based SSO system. This includes personally identifiable information (PII), financial data, intellectual property, or any other confidential information. Data breaches can result in reputational damage, legal consequences, financial losses, and harm to individuals whose data is exposed.

Here is an example of a potential payload:

----
<SAMLResponse>
[...]
<Subject>
<NameID>[email protected]<!---->.evil.com</NameID>
</Subject>
[...]
</SAMLResponse>
----
== How to fix it in Spring

The attacker will manage to generate a valid <SAMLResponse> content with the account "\[email protected]". He will modify it with XML comments to finally be authenticated as "\[email protected]". To prevent this vulnerability on application using Spring Security SAML relying on OpenSAML2, XML comments should be ignored thanks to the property ``++ignoreComments++`` set to ``++true++``.
=== Code examples

The following code examples are vulnerable because they explicitly include comments in signature checks. An attacker is able to change the field identifying the authenticated user with XML comments.

=== Noncompliant code example
==== Noncompliant code example

[source,java]
[source,java,diff-id=1,diff-type=noncompliant]
----
import org.opensaml.xml.parse.BasicParserPool;
import org.opensaml.xml.parse.ParserPool;
import org.opensaml.xml.parse.StaticBasicParserPool;
import org.opensaml.xml.parse.ParserPool;
public ParserPool parserPool() {
StaticBasicParserPool staticBasicParserPool = new StaticBasicParserPool();
staticBasicParserPool.setIgnoreComments(false); // Noncompliant: comments are not ignored during parsing opening the door to exploit the vulnerability
staticBasicParserPool.setIgnoreComments(false); // Noncompliant
return staticBasicParserPool;
}
----

[source,java]
[source,java,diff-id=2,diff-type=noncompliant]
----
import org.opensaml.xml.parse.BasicParserPool;
import org.opensaml.xml.parse.ParserPool;
public ParserPool parserPool() {
BasicParserPool basicParserPool = new BasicParserPool();
basicParserPool.setIgnoreComments(false); // Noncompliant
return basicParserPool;
}
----

==== Compliant solution

=== Compliant solution

[source,java]
[source,java,diff-id=1,diff-type=compliant]
----
import org.opensaml.xml.parse.StaticBasicParserPool;
import org.opensaml.xml.parse.ParserPool;
public ParserPool parserPool() {
return new StaticBasicParserPool(); // Compliant: "ignoreComments" is set to "true" in StaticBasicParserPool constructor
return new StaticBasicParserPool();
}
----

[source,java]
[source,java,diff-id=2,diff-type=compliant]
----
import org.opensaml.xml.parse.BasicParserPool;
import org.opensaml.xml.parse.ParserPool;
public ParserPool parserPool() {
return new BasicParserPool(); // Compliant: "ignoreComments" is set to "true" in BasicParserPool constructor
return new BasicParserPool();
}
----


== Resources

* https://owasp.org/Top10/A06_2021-Vulnerable_and_Outdated_Components/[OWASP Top 10 2021 Category A6] - Vulnerable and Outdated Components
* https://owasp.org/Top10/A07_2021-Identification_and_Authentication_Failures/[OWASP Top 10 2021 Category A7] - Identification and Authentication Failures
* https://owasp.org/www-project-top-ten/2017/A2_2017-Broken_Authentication[OWASP Top 10 2017 Category A2] - Broken Authentication
* https://owasp.org/www-project-top-ten/2017/A9_2017-Using_Components_with_Known_Vulnerabilities[OWASP Top 10 2017 Category A9] - Using Components with Known Vulnerabilities
* https://duo.com/blog/duo-finds-saml-vulnerabilities-affecting-multiple-implementations[Duo Finds SAML Vulnerabilities Affecting Multiple Implementations]
* https://spring.io/blog/2018/03/01/spring-security-saml-and-this-week-s-saml-vulnerability[Spring Security SAML and this week's SAML Vulnerability]
* https://github.com/spring-projects/spring-security-saml/issues/228[Spring Security SAML: Issue #228]
=== Documentation

* OpenSAML API - https://javadoc.io/doc/org.opensaml/xmltooling/latest/org/opensaml/xml/parse/BasicParserPool.html[Class BasicParserPool]
* OpenSAML API - https://javadoc.io/doc/org.opensaml/xmltooling/latest/org/opensaml/xml/parse/StaticBasicParserPool.html[Class StaticBasicParserPool]
* W3C Recommendation - https://www.w3.org/TR/xml-c14n11/[Canonical XML Version 1.1]
* W3C Recommendation - https://www.w3.org/TR/xmldsig-core1/[XML Signature Syntax and Processing Version 1.1]

=== Articles & blog posts

* Cisco Duo - https://duo.com/blog/duo-finds-saml-vulnerabilities-affecting-multiple-implementations[Duo Finds SAML Vulnerabilities Affecting Multiple Implementations]
* Spring blog - https://spring.io/blog/2018/03/01/spring-security-saml-and-this-week-s-saml-vulnerability[Spring Security SAML and this week's SAML Vulnerability]
* Spring Security SAML - https://github.com/spring-projects/spring-security-saml/issues/228[Issue #228 Multiple SAML libraries may allow authentication bypass via incorrect XML canonicalization and DOM traversal]

* CVE - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11427[CVE-2017-11427]
* CVE - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11428[CVE-2017-11428]
* CVE - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11429[CVE-2017-11429]
* CVE - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11430[CVE-2017-11430]
* CVE - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-0489[CVE-2018-0489]
* CVE - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-7340[CVE-2018-7340]

== Standards

* OWASP - https://owasp.org/Top10/A06_2021-Vulnerable_and_Outdated_Components/[Top 10 2021 Category A6 - Vulnerable and Outdated Components]
* OWASP - https://owasp.org/Top10/A07_2021-Identification_and_Authentication_Failures/[Top 10 2021 Category A7 - Identification and Authentication Failures]
* OWASP - https://owasp.org/www-project-top-ten/2017/A9_2017-Using_Components_with_Known_Vulnerabilities[Top 10 2017 Category A9 - Using Components with Known Vulnerabilities]
* OWASP - https://owasp.org/www-project-top-ten/2017/A2_2017-Broken_Authentication[Top 10 2017 Category A2 - Broken Authentication]


ifdef::env-github,rspecator-view[]

Expand Down
50 changes: 50 additions & 0 deletions rules/S5679/metadata.json
Original file line number Diff line number Diff line change
@@ -1,2 +1,52 @@
{
"title": "OpenSAML2 should be configured to prevent authentication bypass",
"type": "VULNERABILITY",
"code": {
"impacts": {
"SECURITY": "MEDIUM"
},
"attribute": "TRUSTWORTHY"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "10min"
},
"tags": [
"spring"
],
"extra": {
"replacementRules": [

],
"legacyKeys": [

]
},
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-5679",
"sqKey": "S5679",
"scope": "Main",
"securityStandards": {
"OWASP": [
"A9",
"A2"
],
"OWASP Top 10 2021": [
"A6",
"A7"
],
"PCI DSS 3.2": [
"6.2",
"6.5.10"
],
"PCI DSS 4.0": [
"6.3.3",
"6.2.4"
]
},
"defaultQualityProfiles": [
"Sonar way"
],
"quickfix": "unknown"
}

0 comments on commit 38c07d1

Please sign in to comment.