Skip to content

Commit

Permalink
Stabilise order of issues slices in MobSF producer tests (#87)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@chrisnovakovic
chrisnovakovic committed Jan 11, 2021
1 parent eb56a67 commit f9b1de7
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 32 deletions.
18 changes: 11 additions & 7 deletions producers/mobsf/report/android/android_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package android
import (
v1 "github.com/thought-machine/dracon/api/proto/v1"

"sort"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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",
Expand All @@ -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)
}

Expand Down
57 changes: 32 additions & 25 deletions producers/mobsf/report/ios/ios_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ios
import (
v1 "github.com/thought-machine/dracon/api/proto/v1"

"sort"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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",
Expand All @@ -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)
}

Expand All @@ -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",
Expand All @@ -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)
}

0 comments on commit f9b1de7

Please sign in to comment.