SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
774 stars 226 forks source link

Fix RuleDescriptorGenerator: the generated `rules.xml` does not contain the rule STATUS #3287

Closed andrei-epure-sonarsource closed 3 years ago

andrei-epure-sonarsource commented 4 years ago

For details see this internal thread.

We upload the rules metadata using a rules.xml file. A rule has the STATUS attribute which, by default, is READY, but it can also be DEPRECATED.

The rules definition are loaded in AbstractRulesDefinition #define using the XML file /org/sonar/plugins/csharp/rules.xml

This file gets generated during the build in e.g. C:\Workspace\sonar-dotnet\sonaranalyzer-dotnet\src\SonarAnalyzer.RuleDescriptorGenerator\bin\Debug\net46\cs

We need to improve the RuleDescriptorGenerator to include the rule status.

andrei-epure-sonarsource commented 4 years ago

the XML looks like

  <rule>
    <key>S4432</key>
    <type>VULNERABILITY</type>
    <name>AES encryption algorithm should be used with secured mode</name>
    <severity>CRITICAL</severity>
    <cardinality>SINGLE</cardinality>
    <description><![CDATA[<p>Encryption algorithms can be used with various modes. Some combinations are not secured:</p>
<ul>
  <li> Electronic Codebook (ECB) mode: Under a given key, any given plaintext block always gets encrypted to the same ciphertext block. Thus, it does
  not hide data patterns well. In some senses, it doesn't provide serious message confidentiality, and it is not recommended for use in cryptographic
  protocols at all. </li>
  <li> Cipher Block Chaining (CBC) with PKCS#5 padding (or PKCS#7) is susceptible to padding oracle attacks. CBC + PKCS#7 can be used if combined with
  an authenticity check (HMAC-SHA256 for example) on the cipher text. </li>
</ul>
<p>In both cases, Galois/Counter Mode (GCM) with no padding should be preferred. As the .NET framework doesn't provide this natively, the use of a
certified third party lib is recommended. </p>
<p>This rule raises an issue when any of the following CipherMode is detected: ECB, CBC, OFB, CFB, CTS.</p>
<h2>Noncompliant Code Example</h2>
<pre>
AesManaged aes = new AesManaged
{
  KeySize = 128,
  BlockSize = 128,
  Mode = CipherMode.OFB, // Noncompliant
  Padding = PaddingMode.PKCS7
};
</pre>
<h2>See</h2>
<ul>
  <li> <a href="https://www.owasp.org/index.php/Top_10-2017_A6-Security_Misconfiguration">OWASP Top 10 2017 Category A6</a> - Security
  Misconfiguration </li>
  <li> <a href="http://cwe.mitre.org/data/definitions/327.html">MITRE, CWE-327</a> - Use of a Broken or Risky Cryptographic Algorithm </li>
  <li> <a href="https://www.securecoding.cert.org/confluence/x/VwAZAg">CERT, MSC61-J.</a> - Do not use insecure or weak cryptographic algorithms </li>
  <li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
  <li> <a href="https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38a.pdf">Recommendation for Block Cipher Modes of Operation</a>
  </li>
  <li> Derived from FindSecBugs rule <a href="https://find-sec-bugs.github.io/bugs.htm#ECB_MODE">ECB_MODE</a> </li>
  <li> Derived from FindSecBugs rule <a href="https://find-sec-bugs.github.io/bugs.htm#PADDING_ORACLE">PADDING_ORACLE</a> </li>
</ul>
<h2>Deprecated</h2>
<p>This rule is deprecated; use {rule:csharpsquid:S5542} instead.</p>

]]></description>
    <remediationFunction>CONSTANT_ISSUE</remediationFunction>
    <remediationFunctionBaseEffort>2min</remediationFunctionBaseEffort>
  </rule>
costin-zaharia-sonarsource commented 3 years ago

While working on this I've noticed that the status for SUPERSEDED rules is not always updated correctly by the rule-api. This PR should provide a fix: https://github.com/SonarSource/sonar-rule-api/pull/117