From f9b1de7fab8b9d88bd89de51d5563d56ea7439fa Mon Sep 17 00:00:00 2001 From: Chris Novakovic Date: Mon, 11 Jan 2021 15:24:39 +0000 Subject: [PATCH] Stabilise order of issues slices in MobSF producer tests (#87) Avoid non-deterministic failures in the MobSF producer tests by sorting issues slices before comparing them with the list of expected issues - the order doesn't actually matter for the purposes of these tests, just the contents of the slices. --- .../mobsf/report/android/android_test.go | 18 +++--- producers/mobsf/report/ios/ios_test.go | 57 +++++++++++-------- 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/producers/mobsf/report/android/android_test.go b/producers/mobsf/report/android/android_test.go index 456ed78..6e25983 100644 --- a/producers/mobsf/report/android/android_test.go +++ b/producers/mobsf/report/android/android_test.go @@ -3,6 +3,7 @@ package android import ( v1 "github.com/thought-machine/dracon/api/proto/v1" + "sort" "testing" "github.com/stretchr/testify/assert" @@ -61,13 +62,6 @@ func TestParseValidIosReportNoExclusions(t *testing.T) { assert.Len(t, issues, 2) expectedIssues := []*v1.Issue{ - &v1.Issue{ - Target: "android_project/test/MainApplication.java:58", - Type: "android_ip_disclosure", - Title: "CWE-200 Information Exposure", - Cvss: 4.3, - Description: "IP Address disclosure", - }, &v1.Issue{ Target: "android_project/test/MainApplication.java:26", Type: "android_insecure_random", @@ -76,8 +70,18 @@ func TestParseValidIosReportNoExclusions(t *testing.T) { Cvss: 7.5, Description: "The App uses an insecure Random Number Generator.", }, + &v1.Issue{ + Target: "android_project/test/MainApplication.java:58", + Type: "android_ip_disclosure", + Title: "CWE-200 Information Exposure", + Cvss: 4.3, + Description: "IP Address disclosure", + }, } + sort.Slice(issues, func(i, j int) bool { + return issues[i].Target < issues[j].Target + }) assert.Equal(t, issues, expectedIssues) } diff --git a/producers/mobsf/report/ios/ios_test.go b/producers/mobsf/report/ios/ios_test.go index e03b434..b58bee6 100644 --- a/producers/mobsf/report/ios/ios_test.go +++ b/producers/mobsf/report/ios/ios_test.go @@ -3,6 +3,7 @@ package ios import ( v1 "github.com/thought-machine/dracon/api/proto/v1" + "sort" "testing" "github.com/stretchr/testify/assert" @@ -81,6 +82,22 @@ func TestParseValidIosReportNoExclusions(t *testing.T) { assert.Len(t, issues, 6) expectedIssues := []*v1.Issue{ + &v1.Issue{ + Target: "ios_project", + Type: "Insecure App Transport Security policy", + Title: "App Transport Security AllowsArbitraryLoads is allowed", + Severity: v1.Severity_SEVERITY_MEDIUM, + Confidence: v1.Confidence_CONFIDENCE_MEDIUM, + Description: "An insecure App Transport Security policy is defined in a plist file in the iOS app project directory ios_project.\n\nDetails:\n\nApp Transport Security AllowsArbitraryLoads is allowed\nApp Transport Security restrictions are disabled for all network connections. Disabling ATS means that unsecured HTTP connections are allowed. HTTPS connections are also allowed, and are still subject to default server trust evaluation. However, extended security checks like requiring a minimum Transport Layer Security (TLS) protocol version\342\200\224are disabled. This setting is not applicable to domains listed in NSExceptionDomains.", + }, + &v1.Issue{ + Target: "ios_project", + Type: "Insecure App Transport Security policy", + Title: "NSExceptionRequiresForwardSecrecy set to NO for localhost", + Severity: v1.Severity_SEVERITY_MEDIUM, + Confidence: v1.Confidence_CONFIDENCE_MEDIUM, + Description: "An insecure App Transport Security policy is defined in a plist file in the iOS app project directory ios_project.\n\nDetails:\n\nNSExceptionRequiresForwardSecrecy set to NO for localhost\nNSExceptionRequiresForwardSecrecy limits the accepted ciphers to those that support perfect forward secrecy (PFS) through the Elliptic Curve Diffie-Hellman Ephemeral (ECDHE) key exchange. Set the value for this key to NO to override the requirement that a server must support PFS for the given domain. This key is optional. The default value is YES, which limits the accepted ciphers to those that support PFS through Elliptic Curve Diffie-Hellman Ephemeral (ECDHE) key exchange.", + }, &v1.Issue{ Target: "ios_project/file.m:31", Type: "ios_app_logging", @@ -96,37 +113,24 @@ func TestParseValidIosReportNoExclusions(t *testing.T) { Description: "The App logs information. Sensitive information should never be logged.", }, &v1.Issue{ - Target: "ios_project/file2.swift:37", + Target: "ios_project/file2.swift:16", Type: "ios_swift_log", Title: "CWE-532", Cvss: 7.5, Description: "The App logs information. Sensitive information should never be logged.", }, &v1.Issue{ - Target: "ios_project/file2.swift:16", + Target: "ios_project/file2.swift:37", Type: "ios_swift_log", Title: "CWE-532", Cvss: 7.5, Description: "The App logs information. Sensitive information should never be logged.", }, - &v1.Issue{ - Target: "ios_project", - Type: "Insecure App Transport Security policy", - Title: "App Transport Security AllowsArbitraryLoads is allowed", - Severity: v1.Severity_SEVERITY_MEDIUM, - Confidence: v1.Confidence_CONFIDENCE_MEDIUM, - Description: "An insecure App Transport Security policy is defined in a plist file in the iOS app project directory ios_project.\n\nDetails:\n\nApp Transport Security AllowsArbitraryLoads is allowed\nApp Transport Security restrictions are disabled for all network connections. Disabling ATS means that unsecured HTTP connections are allowed. HTTPS connections are also allowed, and are still subject to default server trust evaluation. However, extended security checks like requiring a minimum Transport Layer Security (TLS) protocol version\342\200\224are disabled. This setting is not applicable to domains listed in NSExceptionDomains.", - }, - &v1.Issue{ - Target: "ios_project", - Type: "Insecure App Transport Security policy", - Title: "NSExceptionRequiresForwardSecrecy set to NO for localhost", - Severity: v1.Severity_SEVERITY_MEDIUM, - Confidence: v1.Confidence_CONFIDENCE_MEDIUM, - Description: "An insecure App Transport Security policy is defined in a plist file in the iOS app project directory ios_project.\n\nDetails:\n\nNSExceptionRequiresForwardSecrecy set to NO for localhost\nNSExceptionRequiresForwardSecrecy limits the accepted ciphers to those that support perfect forward secrecy (PFS) through the Elliptic Curve Diffie-Hellman Ephemeral (ECDHE) key exchange. Set the value for this key to NO to override the requirement that a server must support PFS for the given domain. This key is optional. The default value is YES, which limits the accepted ciphers to those that support PFS through Elliptic Curve Diffie-Hellman Ephemeral (ECDHE) key exchange.", - }, } + sort.Slice(issues, func(i, j int) bool { + return issues[i].Target < issues[j].Target || issues[i].Description < issues[j].Description + }) assert.Equal(t, issues, expectedIssues) } @@ -139,13 +143,6 @@ func TestParseValidIosReportExclusions(t *testing.T) { assert.Len(t, issues, 3) expectedIssues := []*v1.Issue{ - &v1.Issue{ - Target: "ios_project/file.m:31", - Type: "ios_app_logging", - Title: "CWE-532 Insertion of Sensitive Information into Log File", - Cvss: 7.5, - Description: "The App logs information. Sensitive information should never be logged.", - }, &v1.Issue{ Target: "ios_project", Type: "Insecure App Transport Security policy", @@ -162,7 +159,17 @@ func TestParseValidIosReportExclusions(t *testing.T) { Confidence: v1.Confidence_CONFIDENCE_MEDIUM, Description: "An insecure App Transport Security policy is defined in a plist file in the iOS app project directory ios_project.\n\nDetails:\n\nNSExceptionRequiresForwardSecrecy set to NO for localhost\nNSExceptionRequiresForwardSecrecy limits the accepted ciphers to those that support perfect forward secrecy (PFS) through the Elliptic Curve Diffie-Hellman Ephemeral (ECDHE) key exchange. Set the value for this key to NO to override the requirement that a server must support PFS for the given domain. This key is optional. The default value is YES, which limits the accepted ciphers to those that support PFS through Elliptic Curve Diffie-Hellman Ephemeral (ECDHE) key exchange.", }, + &v1.Issue{ + Target: "ios_project/file.m:31", + Type: "ios_app_logging", + Title: "CWE-532 Insertion of Sensitive Information into Log File", + Cvss: 7.5, + Description: "The App logs information. Sensitive information should never be logged.", + }, } + sort.Slice(issues, func(i, j int) bool { + return issues[i].Target < issues[j].Target || issues[i].Description < issues[j].Description + }) assert.Equal(t, issues, expectedIssues) }