Skip to content

Commit

Permalink
Update RSPEC before 9.12 release (#8161)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregory-paidis-sonarsource authored Oct 9, 2023
1 parent f6ea225 commit 3786561
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 139 deletions.
2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S1854.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@
563
]
},
"quickfix": "unknown"
"quickfix": "targeted"
}
16 changes: 14 additions & 2 deletions analyzers/rspec/cs/S3241.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
<h2>Why is this an issue?</h2>
<p>Private methods are clearly intended for use only within their own scope. When such methods return values that are never used by any of their
callers, then clearly there is no need to actually make the return, and it should be removed in the interests of efficiency and clarity.</p>
<p>Private methods are intended for use only within their scope. If these methods return values that are not utilized by any calling functions, it
indicates that the return operation is unnecessary. Removing such returns can enhance both efficiency and code clarity.</p>
<h3>Noncompliant code example</h3>
<pre>
class SomeClass
{
private int PrivateMethod() =&gt; 42;

public void PublicMethod()
{
PrivateMethod(); // Noncompliant: the result of PrivateMethod is not used
}
}
</pre>

5 changes: 5 additions & 0 deletions analyzers/rspec/cs/S3776.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,9 @@ <h3>Documentation</h3>
<ul>
<li> <a href="https://www.sonarsource.com/docs/CognitiveComplexity.pdf">Cognitive Complexity</a> </li>
</ul>
<h3>Articles &amp; blog posts</h3>
<ul>
<li> <a href="https://www.sonarsource.com/blog/5-clean-code-tips-for-reducing-cognitive-complexity/">5 Clean Code Tips for Reducing Cognitive
Complexity</a> </li>
</ul>

158 changes: 91 additions & 67 deletions analyzers/rspec/cs/S5773.html
Original file line number Diff line number Diff line change
@@ -1,63 +1,63 @@
<p>Deserialization is the process of converting serialized data (such as objects or data structures) back into their original form. Types allowed to
be unserialized should be strictly controlled.</p>
<h2>Why is this an issue?</h2>
<p>During the deserialization process, the state of an object will be reconstructed from the serialized data stream which can contain dangerous
operations.</p>
<p>During the deserialization process, the state of an object will be reconstructed from the serialized data stream. By allowing unrestricted
deserialization of types, the application makes it possible for attackers to use types with dangerous or otherwise sensitive behavior during the
deserialization process.</p>
<h3>What is the potential impact?</h3>
<p>When an application deserializes untrusted data without proper restrictions, an attacker can craft malicious serialized objects. Depending on the
affected objects and properties, the consequences can vary.</p>
<h3>Remote Code Execution</h3>
<p>If attackers can craft malicious serialized objects that contain executable code, this code will run within the application’s context, potentially
gaining full control over the system. This can lead to unauthorized access, data breaches, or even complete system compromise.</p>
<p>For example, a well-known attack vector consists in serializing an object of type <code><a
href="https://docs.microsoft.com/en-us/dotnet/api/system.codedom.compiler.tempfilecollection.-ctor?view=netframework-4.8#System_CodeDom_Compiler_TempFileCollection__ctor">TempFileCollection</a></code>
with arbitrary files (defined by an attacker) which will be deleted on the application deserializing this object (when the <a
href="https://docs.microsoft.com/en-us/dotnet/api/system.codedom.compiler.tempfilecollection.finalize?view=netframework-4.8">finalize() </a>method of
the TempFileCollection object is called). This kind of types are called "<a href="https://github.com/pwntester/ysoserial.net">gadgets</a>".</p>
<p>Instead of using <code>BinaryFormatter</code> and similar serializers, it is recommended to use safer alternatives in most of the cases, such as <a
href="https://docs.microsoft.com/en-us/dotnet/api/system.xml.serialization.xmlserializer?view=net-5.0">XmlSerializer</a> or <a
href="https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.datacontractserializer?view=net-5.0">DataContractSerializer</a>. If
it’s not possible then try to mitigate the risk by restricting the types allowed to be deserialized:</p>
<ul>
<li> by implementing an "allow-list" of types, but keep in mind that novel dangerous types are regularly discovered and this protection could be
insufficient over time. </li>
<li> or/and implementing a tamper protection, such as <a href="https://en.wikipedia.org/wiki/HMAC">message authentication codes</a> (MAC). This way
only objects serialized with the correct MAC hash will be deserialized. </li>
</ul>
<h3>Noncompliant code example</h3>
<p>For <a
href="https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter?view=netframework-4.8">BinaryFormatter</a>,
<a
href="https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.netdatacontractserializer?view=netframework-4.8">NetDataContractSerializer</a>,
href="https://docs.microsoft.com/en-us/dotnet/api/system.codedom.compiler.tempfilecollection.finalize?view=netframework-4.8">finalize()</a> method of
the TempFileCollection object is called). These kinds of specially crafted serialized objects are called "<a
href="https://github.com/pwntester/ysoserial.net">gadgets</a>".</p>
<h3>Privilege escalation</h3>
<p>Unrestricted deserialization can also enable attackers to escalate their privileges within the application. By manipulating the serialized data, an
attacker can modify object properties or bypass security checks, granting them elevated privileges that they should not have. This can result in
unauthorized access to sensitive data, unauthorized actions, or even administrative control over the application.</p>
<h3>Denial of Service</h3>
<p>In some cases, an attacker can abuse the deserialization process to cause a denial of service (DoS) condition. By providing specially crafted
serialized data, the attacker can trigger excessive resource consumption, leading to system instability or unresponsiveness. This can disrupt the
availability of the application, impacting its functionality and causing inconvenience to users.</p>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<p>With <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter"><code>BinaryFormatter</code></a>,
<a
href="https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.soap.soapformatter?view=netframework-4.8">SoapFormatter</a>
serializers:</p>
<pre>
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.netdatacontractserializer"><code>NetDataContractSerializer</code></a>
or <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.soap.soapformatter"><code>SoapFormatter</code></a>:</p>
<pre data-diff-id="101" data-diff-type="noncompliant">
var myBinaryFormatter = new BinaryFormatter();
myBinaryFormatter.Deserialize(stream); // Noncompliant: a binder is not used to limit types during deserialization
myBinaryFormatter.Deserialize(stream); // Noncompliant
</pre>
<p><a
href="https://docs.microsoft.com/en-us/dotnet/api/system.web.script.serialization.javascriptserializer?view=netframework-4.8">JavaScriptSerializer</a>
should not use SimpleTypeResolver or other weak resolvers:</p>
<pre>
JavaScriptSerializer serializer1 = new JavaScriptSerializer(new SimpleTypeResolver()); // Noncompliant: SimpleTypeResolver is unsecure (every types is resolved)
<p>With <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.web.script.serialization.javascriptserializer"><code>JavaScriptSerializer</code></a>:</p>
<pre data-diff-id="102" data-diff-type="noncompliant">
JavaScriptSerializer serializer1 = new JavaScriptSerializer(new SimpleTypeResolver()); // Noncompliant
serializer1.Deserialize&lt;ExpectedType&gt;(json);
</pre>
<p><a href="https://docs.microsoft.com/en-us/dotnet/api/system.web.ui.losformatter?view=netframework-4.8">LosFormatter</a> should not be used without
MAC verification:</p>
<pre>
LosFormatter formatter = new LosFormatter(); // Noncompliant
formatter.Deserialize(fs);
</pre>
<h3>Compliant solution</h3>
<p><a
href="https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter?view=netframework-4.8">BinaryFormatter</a>,
<h4>Compliant solution</h4>
<p>With <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter"><code>BinaryFormatter</code></a>,
<a
href="https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.netdatacontractserializer?view=netframework-4.8">NetDataContractSerializer
</a>, <a
href="https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.soap.soapformatter?view=netframework-4.8">SoapFormatter</a>
serializers should use a binder implementing a whitelist approach to limit types during deserialization (at least one exception should be thrown or a
null value returned):</p>
<pre>
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.netdatacontractserializer"><code>NetDataContractSerializer</code></a>
or <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.soap.soapformatter"><code>SoapFormatter</code></a>:</p>
<pre data-diff-id="101" data-diff-type="compliant">
sealed class CustomBinder : SerializationBinder
{
public override Type BindToType(string assemblyName, string typeName)
{
if (!(typeName == "type1" || typeName == "type2" || typeName == "type3"))
{
throw new SerializationException("Only type1, type2 and type3 are allowed"); // Compliant
throw new SerializationException("Only type1, type2 and type3 are allowed");
}
return Assembly.Load(assemblyName).GetType(typeName);
}
Expand All @@ -67,43 +67,67 @@ <h3>Compliant solution</h3>
myBinaryFormatter.Binder = new CustomBinder();
myBinaryFormatter.Deserialize(stream);
</pre>
<p><a
href="https://docs.microsoft.com/en-us/dotnet/api/system.web.script.serialization.javascriptserializer?view=netframework-4.8">JavaScriptSerializer</a>
should use a resolver implementing a whitelist to limit types during deserialization (at least one exception should be thrown or a null value
returned):</p>
<pre>
<p>With <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.web.script.serialization.javascriptserializer"><code>JavaScriptSerializer</code></a>:</p>
<pre data-diff-id="102" data-diff-type="compliant">
public class CustomSafeTypeResolver : JavaScriptTypeResolver
{
public override Type ResolveType(string id)
{
if(id != "ExpectedType") {
throw new ArgumentNullException("Only ExpectedType is allowed during deserialization"); // Compliant
throw new ArgumentNullException("Only ExpectedType is allowed during deserialization");
}
return Type.GetType(id);
}
}

JavaScriptSerializer serializer = new JavaScriptSerializer(new CustomSafeTypeResolver()); // Compliant
JavaScriptSerializer serializer = new JavaScriptSerializer(new CustomSafeTypeResolver());
serializer.Deserialize&lt;ExpectedType&gt;(json);
</pre>
<p><a href="https://docs.microsoft.com/en-us/dotnet/api/system.web.ui.losformatter?view=netframework-4.8">LosFormatter</a> serializer with MAC
verification:</p>
<pre>
LosFormatter formatter = new LosFormatter(true, secret); // Compliant
formatter.Deserialize(fs);
</pre>
<h3>Going the extra mile</h3>
<p>Instead of using <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter"><code>BinaryFormatter</code></a>
and similar serializers, it is recommended to use safer alternatives in most of the cases, such as <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.xml.serialization.xmlserializer"><code>XmlSerializer</code></a> or <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.datacontractserializer"><code>DataContractSerializer</code></a>.</p>
<p>If it’s not possible then try to mitigate the risk by restricting the types allowed to be deserialized:</p>
<ul>
<li> by implementing an "allow-list" of types, but keep in mind that novel dangerous types are regularly discovered and this protection could be
insufficient over time. </li>
<li> or/and implementing a tamper protection, such as <a href="https://en.wikipedia.org/wiki/HMAC">message authentication codes</a> (MAC). This way
only objects serialized with the correct MAC hash will be deserialized. </li>
</ul>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures/">OWASP Top 10 2021 Category A8</a> - Software and Data
Integrity Failures </li>
<li> <a href="https://docs.microsoft.com/en-us/dotnet/standard/serialization/binaryformatter-security-guide?s=03">docs.microsoft.com</a> -
BinaryFormatter security guide </li>
<li> <a href="https://owasp.org/www-project-top-ten/2017/A8_2017-Insecure_Deserialization">OWASP Top 10 2017 Category A8</a> - Insecure
Deserialization </li>
<li> <a href="https://cwe.mitre.org/data/definitions/134">MITRE, CWE-134</a> - Use of Externally-Controlled Format String </li>
<li> <a href="https://cwe.mitre.org/data/definitions/502">MITRE, CWE-502</a> - Deserialization of Untrusted Data </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat2">SANS Top 25</a> - Risky Resource Management </li>
<li> <a href="https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Deserialization_Cheat_Sheet.md">OWASP Deserialization Cheat
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter">BinaryFormatter Class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.netdatacontractserializer">NetDataContractSerializer Class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.soap.soapformatter">SoapFormatter Class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.web.script.serialization.javascriptserializer">JavaScriptSerializer Class</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.xml.serialization.xmlserializer">XmlSerializer Class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.datacontractserializer">DataContractSerializer Class</a> </li>
</ul>
<h3>Articles &amp; blog posts</h3>
<ul>
<li> Microsoft Learn - <a href="https://docs.microsoft.com/en-us/dotnet/standard/serialization/binaryformatter-security-guide?s=03">Deserialization
risks in use of BinaryFormatter and related types</a> </li>
<li> OWASP - <a href="https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Deserialization_Cheat_Sheet.md">Deserialization Cheat
Sheet</a> </li>
<li> Wikipedia - <a href="https://en.wikipedia.org/wiki/HMAC">Message Authentication Codes (MAC)</a> </li>
</ul>
<h3>Standards</h3>
<ul>
<li> OWASP - <a href="https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures/">Top 10 2021 Category A8 - Software and Data Integrity
Failures</a> </li>
<li> OWASP - <a href="https://owasp.org/www-project-top-ten/2017/A8_2017-Insecure_Deserialization">Top 10 2017 Category A8 - Insecure
Deserialization</a> </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/134">CWE-134 - Use of Externally-Controlled Format String</a> </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/502">CWE-502 - Deserialization of Untrusted Data</a> </li>
<li> SANS - <a href="https://www.sans.org/top25-software-errors/#cat2">Top 25 - Risky Resource Management</a> </li>
</ul>

5 changes: 5 additions & 0 deletions analyzers/rspec/vbnet/S3776.html
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,9 @@ <h3>Documentation</h3>
<ul>
<li> <a href="https://www.sonarsource.com/docs/CognitiveComplexity.pdf">Cognitive Complexity</a> </li>
</ul>
<h3>Articles &amp; blog posts</h3>
<ul>
<li> <a href="https://www.sonarsource.com/blog/5-clean-code-tips-for-reducing-cognitive-complexity/">5 Clean Code Tips for Reducing Cognitive
Complexity</a> </li>
</ul>

Loading

0 comments on commit 3786561

Please sign in to comment.