Skip to content

Commit

Permalink
Improving rule accuracy (#523)
Browse files Browse the repository at this point in the history
  • Loading branch information
atrompler authored Jun 9, 2023
1 parent 6f4918e commit bc00919
Show file tree
Hide file tree
Showing 14 changed files with 251 additions and 29 deletions.
6 changes: 5 additions & 1 deletion Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [1.0.8] - 2023-06-09
### Rules
- Adds new rules and improves precision of some existing rules.

## [1.0.7] - 2023-06-06
### Fixed
- Fixes issue where the CLI global tool package was attempting to run with a mismatched runtime.
Expand Down Expand Up @@ -65,4 +69,4 @@ New: `devskim analyze -I path/to/src -O path/to/out.sarif`
- Pack, test and catalogue commands removed from CLI

### Fixes
- Rule improvements and DevSkim engine performance and reliablity improvements.
- Rule improvements and DevSkim engine performance and reliablity improvements.
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
<ItemGroup>
<PackageReference Include="CommandLineParser" Version="2.9.1" />
<PackageReference Include="LibGit2Sharp" Version="0.27.2" />
<PackageReference Include="Microsoft.CST.ApplicationInspector.Logging" Version="1.8.7" />
<PackageReference Include="Microsoft.CST.ApplicationInspector.Logging" Version="1.9.3" />
<PackageReference Include="Microsoft.Extensions.CommandLineUtils" Version="1.1.1" />
<PackageReference Include="Sarif.Sdk" Version="4.1.0" />
<PackageReference Include="Sarif.Sdk" Version="4.2.0" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@

<ItemGroup>
<PackageReference Include="CommandLineParser" Version="2.9.1" />
<PackageReference Include="Microsoft.CST.ApplicationInspector.RulesEngine" Version="1.8.7" />
<PackageReference Include="Microsoft.CST.ApplicationInspector.RulesEngine" Version="1.9.3" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="7.0.0" />
<PackageReference Include="OmniSharp.Extensions.LanguageServer" Version="0.19.7" />
<PackageReference Include="Serilog.Extensions.Logging" Version="3.1.0" />
<PackageReference Include="Serilog.Extensions.Logging" Version="7.0.0" />
<PackageReference Include="Serilog.Sinks.Debug" Version="2.0.0" />
<PackageReference Include="Serilog.Sinks.File" Version="5.0.0" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,9 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
<PackageReference Include="MSTest.TestAdapter" Version="3.0.2" />
<PackageReference Include="MSTest.TestFramework" Version="3.0.2" />
<PackageReference Include="coverlet.collector" Version="3.2.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.2" />
<PackageReference Include="MSTest.TestAdapter" Version="3.0.4" />
<PackageReference Include="MSTest.TestFramework" Version="3.0.4" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Common" Version="4.5.0" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" Version="4.6.0" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@
</None>
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.5.0" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.6.0" />
<PackageReference Include="Microsoft.VisualStudio.LanguageServer.Client">
<Version>17.5.62</Version>
<Version>17.6.42</Version>
</PackageReference>
<PackageReference Include="Microsoft.VisualStudio.LanguageServer.Protocol">
<Version>17.2.8</Version>
</PackageReference>
<PackageReference Include="Microsoft.VisualStudio.SDK" Version="17.5.33428.388" ExcludeAssets="runtime">
<PackageReference Include="Microsoft.VisualStudio.SDK" Version="17.6.36389" ExcludeAssets="runtime">
<IncludeAssets>compile; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.VSSDK.BuildTools" Version="17.5.4074">
<PackageReference Include="Microsoft.VSSDK.BuildTools" Version="17.6.2164">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
Expand Down
2 changes: 1 addition & 1 deletion DevSkim-DotNet/Microsoft.DevSkim/Microsoft.DevSkim.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CST.ApplicationInspector.RulesEngine" Version="1.8.7" />
<PackageReference Include="Microsoft.CST.ApplicationInspector.RulesEngine" Version="1.9.3" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
</ItemGroup>

Expand Down
12 changes: 12 additions & 0 deletions guidance/DS132781.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## XSLT scripting should be disabled if possible

### Summary
XSLT scripting contains significant security risks if used on untrustworthy data.

### Details
TO DO - put more details of problem and solution here

### References

*[https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ms763800(v=vs.85)](https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ms763800(v=vs.85))
* [https://learn.microsoft.com/en-us/dotnet/api/system.xml.xsl.xsltsettings.enablescript?view=net-8.0](https://learn.microsoft.com/en-us/dotnet/api/system.xml.xsl.xsltsettings.enablescript?view=net-8.0)
17 changes: 17 additions & 0 deletions guidance/DS440001.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
## Hard-coded Cryptography

### Summary

Usage of hard-coded cryptography reduces the crypto agility of an application, which can hinder attempts to move away from vulnerable cryptography.

### Details

Crypto agility is the ability of an application to migrate from one cryptographic algorithm to another. This ability is valuable both in situations where new, stronger cryptographic algorithms become available, and new attacks against existing cryptographic algorithms become known, making those algorithms no longer safe to use.

Crypto agility includes being able to switch parameters of an existing algorithm, such as switching between AES-128 and AES-256.

In order to facilitate this ability, hard-coding crypto algorithms and parameters is not recommended.

### References

* [https://learn.microsoft.com/en-us/archive/msdn-magazine/2009/brownfield/cryptographic-agility](https://learn.microsoft.com/en-us/archive/msdn-magazine/2009/brownfield/cryptographic-agility)
18 changes: 9 additions & 9 deletions rules/default/security/TLS/tls_generic.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"DS440000"
],
"severity": "ManualReview",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "\\b(SSL|D?TLS) ?v?[0123][0123_\\.]+",
Expand Down Expand Up @@ -60,7 +60,7 @@
],
"severity": "ManualReview",
"_comment": "Applies to all languages since many just wrap OpenSSL constructs.",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "SSLv2?3_method|D?TLSv1_([123]_)?(client_|server_)?method",
Expand Down Expand Up @@ -129,7 +129,7 @@
],
"severity": "ManualReview",
"_comment": "Applies to all languages since many just wrap OpenSSL constructs.",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "\\b(SSL|D?TLS) ?v?[0123_\\.]*_VERSION(_MAJOR)?",
Expand Down Expand Up @@ -158,7 +158,7 @@
],
"severity": "ManualReview",
"_comment": "Applies to all languages since many just wrap GnuTLS constructs.",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "GNUTLS_(SSL|D?TLS)[01239_\\.]+",
Expand Down Expand Up @@ -187,7 +187,7 @@
],
"severity": "ManualReview",
"_comment": "Applies to all languages since many just wrap LibreSSL constructs.",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "(SSL|D?TLS)[01239_\\.]+_VERSION(_MAJOR|_MINOR)?",
Expand Down Expand Up @@ -216,7 +216,7 @@
],
"severity": "ManualReview",
"_comment": "Applies to all languages since many just wrap mbedTLS constructs.",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "MBEDTLS_SSL_PROTO_(SSL|D?TLS)[123_]+",
Expand Down Expand Up @@ -252,7 +252,7 @@
"Cryptography.Protocol.TLS.Hard-Coded"
],
"severity": "ManualReview",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "--(sslv2|sslv3|tlsv1|tlsv11|tlsv1\\.1|tlsv1\\.2)",
Expand Down Expand Up @@ -294,7 +294,7 @@
"Cryptography.Protocol.TLS.Hard-Coded"
],
"severity": "ManualReview",
"rule_info": "DS440000.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "--secure-protocol=",
Expand Down Expand Up @@ -365,7 +365,7 @@
"Cryptography.Protocol.TLS.Elliptic-Curve.Hard-Coded"
],
"severity": "ManualReview",
"rule_info": "DS440100.md",
"rule_info": "DS440001.md",
"patterns": [
{
"pattern": "arbitrary_explicit_char2_curves|arbitrary_explicit_prime_curves|brainpoolP160r1|brainpoolP160t1|brainpoolP192r1|brainpoolP192t1|brainpoolP224r1|brainpoolP224t1|brainpoolP256r1|brainpoolp256r1|brainpoolP256t1|brainpoolP320r1|brainpoolP320t1|brainpoolP384r1|brainpoolp384r1|brainpoolP384t1|brainpoolP512r1|brainpoolp512r1|brainpoolP512t1|curve25519|ec192wapi|nistP192|nistP224|nistP256|nistP384|nistP521|numsP256t1|numsP384t1|numsP512t1|secP160k1|secp160k1|secP160r1|secp160r1|secP160r2|secp160r2|secP192k1|secp192k1|secP192r1|secp192r1|secP224k1|secp224k1|secP224r1|secp224r1|secP256k1|secp256k1|secP256r1|secp256r1|secP384r1|secp384r1|secP521r1|secp521r1|sect163k1|sect163r1|sect163r2|sect193r1|sect193r2|sect233k1|sect233r1|sect239k1|sect283k1|sect283r1|sect409k1|sect409r1|sect571k1|sect571r1|wtls12|wtls7|wtls9|X25519|X448|x962P192v1|x962P192v2|x962P192v3|x962P239v1|x962P239v2|x962P239v3|x962P256v1",
Expand Down
11 changes: 9 additions & 2 deletions rules/default/security/cryptography/hardcoded_tls.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,17 @@
],
"severity": "important",
"rule_info": "DS440000.md",
"must-not-match": ["LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCg==", "curl --tlsv1.3"],
"must-match": [
"SecurityProtocolType.Tls11"
],
"must-not-match": [
"LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCg==",
"curl --tlsv1.3",
"axolotls"
],
"patterns": [
{
"pattern": "(SSL|D?TLS)[_v]*[123]([123_\\.])*",
"pattern": "(?<=\\W)(SSL|D?TLS)[_v]*[123]([123_\\.])*",
"type": "regex",
"scopes": [
"code"
Expand Down
1 change: 1 addition & 0 deletions rules/default/security/cryptography/hash_algorithm.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"tags": [
"Cryptography.BannedHashAlgorithm"
],
"does_not_apply_to": ["json"],
"severity": "critical",
"rule_info": "DS126858.md",
"patterns": [
Expand Down
136 changes: 136 additions & 0 deletions rules/default/security/frameworks/android.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
[
{
"name": "Android debug is enabled.",
"id": "DS180000",
"description": "The android:debuggable element is set to true, which should be disabled for release builds.",
"recommendation": "Set android:debuggable to false for release builds.",
"applies_to_file_regex": [
"AndroidManifest.xml"
],
"tags": [
"Framework.Android"
],
"severity": "ManualReview",
"rule_info": "DS180000.md",
"patterns": [
{
"xpaths": ["//default:application/@android:debuggable"],
"xpathnamespaces": {
"default": "http://maven.apache.org/POM/4.0.0",
"android": "http://schemas.android.com/apk/res/android"
}
"pattern": "true",
"type": "regex",
"scopes": [
"code"
],
"modifiers" : ["i"]
}
],
"fix_its": [
{
"name": "Change to false",
"type": "RegexReplace",
"replacement": "false",
"pattern": {
"pattern": "true",
"type": "regex",
"scopes": [
"code"
],
"_comment": ""
}
}
],
"must-match": [
"<?xml version=\"1.0\" encoding=\"utf-8\"?><manifest xmlns=\"http://maven.apache.org/POM/4.0.0\" xmlns:android=\"http://schemas.android.com/apk/res/android\"><application android:debuggable='true' /></manifest>"
],
"must-not-match": [
"<?xml version=\"1.0\" encoding=\"utf-8\"?><manifest xmlns=\"http://maven.apache.org/POM/4.0.0\" xmlns:android=\"http://schemas.android.com/apk/res/android\"><application android:debuggable='false' /></manifest>"
]
},
{
"name": "Android debug is enabled.",
"id": "DS180001",
"description": "The setWebContentsDebuggingEnabled element is set to true, which should be disabled for release builds.",
"recommendation": "Set setWebContentsDebuggingEnabled to false for release builds.",
"applies_to": [
"java"
],
"tags": [
"Framework.Android"
],
"severity": "ManualReview",
"rule_info": "DS180000.md",
"patterns": [
{
"pattern": "setWebContentsDebuggingEnabled\\(true\\)",
"type": "regex",
"scopes": [
"code"
],
"modifiers" : ["i"]
}
],
"fix_its": [
{
"name": "Change to false",
"type": "RegexReplace",
"replacement": "false",
"pattern": {
"pattern": "true",
"type": "regex",
"scopes": [
"code"
],
"_comment": ""
}
}
],
"must-match": [
"setWebContentsDebuggingEnabled(true)"
],
"must-not-match": [
"setWebContentsDebuggingEnabled(false)"
]
},
{
"name": "Android StrictMode is enabled.",
"id": "DS180002",
"description": "StrictMode is detected, which is useful for developers but should be disabled for release builds.",
"recommendation": "Disable StrictMode for release builds.",
"applies_to": [
"java"
],
"tags": [
"Framework.Android"
],
"severity": "ManualReview",
"rule_info": "DS180001.md",
"patterns": [
{
"pattern": "StrictMode.setThreadPolicy(",
"type": "substring",
"scopes": [
"code"
],
"modifiers" : ["i"]
},
{
"pattern": "StrictMode.setVmPolicy(",
"type": "substring",
"scopes": [
"code"
],
"modifiers" : ["i"]
}
],
"must-match": [
"StrictMode.setThreadPolicy(new StrictMode.ThreadPolicy.Builder()",
"StrictMode.setVmPolicy(new StrictMode.VmPolicy.Builder()"
],
"must-not-match": [
"application(StrictMode)"
]
}
]
Loading

0 comments on commit bc00919

Please sign in to comment.