From 49f47fb164123edd14400c25404fd5bc6c951f37 Mon Sep 17 00:00:00 2001 From: Yevhen Zavhorodnii Date: Thu, 13 Jun 2024 12:59:49 +0100 Subject: [PATCH] Fix sql no sql injection rule --- .gitignore | 4 ++-- pkg/risks/builtin/sql_nosql_injection_rule.go | 4 ++++ .../builtin/sql_nosql_injection_rule_test.go | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index f932bbcb..ef917cf4 100644 --- a/.gitignore +++ b/.gitignore @@ -8,7 +8,7 @@ tags.xlsx risks.json technical-assets.json stats.json - +.vscode # Binaries for programs and plugins *.exe @@ -30,4 +30,4 @@ stats.json # IDE stuff /.idea/ -/config.json \ No newline at end of file +/config.json diff --git a/pkg/risks/builtin/sql_nosql_injection_rule.go b/pkg/risks/builtin/sql_nosql_injection_rule.go index a4720b27..e5b91ed7 100644 --- a/pkg/risks/builtin/sql_nosql_injection_rule.go +++ b/pkg/risks/builtin/sql_nosql_injection_rule.go @@ -42,6 +42,10 @@ func (r *SqlNoSqlInjectionRule) GenerateRisks(input *types.Model) ([]*types.Risk risks := make([]*types.Risk, 0) for _, id := range input.SortedTechnicalAssetIDs() { technicalAsset := input.TechnicalAssets[id] + if technicalAsset.OutOfScope || technicalAsset.Type != types.Datastore { + continue + } + incomingFlows := input.IncomingTechnicalCommunicationLinksMappedByTargetId[technicalAsset.Id] for _, incomingFlow := range incomingFlows { potentialDatabaseAccessProtocol := incomingFlow.Protocol.IsPotentialDatabaseAccessProtocol() diff --git a/pkg/risks/builtin/sql_nosql_injection_rule_test.go b/pkg/risks/builtin/sql_nosql_injection_rule_test.go index d882fef0..b63eb322 100644 --- a/pkg/risks/builtin/sql_nosql_injection_rule_test.go +++ b/pkg/risks/builtin/sql_nosql_injection_rule_test.go @@ -43,6 +43,7 @@ type SqlNoSqlInjectionRuleTest struct { confidentiality types.Confidentiality integrity types.Criticality usage types.Usage + assetType types.TechnicalAssetType protocol types.Protocol isVulnerableToQueryInjection bool @@ -55,23 +56,33 @@ type SqlNoSqlInjectionRuleTest struct { func TestSqlNoSqlInjectionRuleCreateRisks(t *testing.T) { testCases := map[string]SqlNoSqlInjectionRuleTest{ "not database protocol": { + assetType: types.Datastore, protocol: types.SmbEncrypted, expectRiskCreated: false, isVulnerableToQueryInjection: true, }, "not vulnerable to query injection not lax": { + assetType: types.Datastore, protocol: types.JdbcEncrypted, expectRiskCreated: false, isVulnerableToQueryInjection: false, }, "lax database always vulnerable to query injection": { + assetType: types.Datastore, protocol: types.HTTP, isVulnerableToQueryInjection: false, expectRiskCreated: true, expectedLikelihood: types.VeryLikely, expectedImpact: types.MediumImpact, }, + "no datastore": { + assetType: types.Process, + protocol: types.JdbcEncrypted, + isVulnerableToQueryInjection: true, + expectRiskCreated: false, + }, "database protocol and vulnerable to query injection": { + assetType: types.Datastore, protocol: types.JdbcEncrypted, expectRiskCreated: true, isVulnerableToQueryInjection: true, @@ -79,6 +90,7 @@ func TestSqlNoSqlInjectionRuleCreateRisks(t *testing.T) { expectedImpact: types.MediumImpact, }, "strictly confidential tech asset high impact": { + assetType: types.Datastore, protocol: types.JdbcEncrypted, expectRiskCreated: true, isVulnerableToQueryInjection: true, @@ -88,6 +100,7 @@ func TestSqlNoSqlInjectionRuleCreateRisks(t *testing.T) { expectedImpact: types.HighImpact, }, "mission critical integrity tech asset high impact": { + assetType: types.Datastore, protocol: types.JdbcEncrypted, expectRiskCreated: true, isVulnerableToQueryInjection: true, @@ -97,6 +110,7 @@ func TestSqlNoSqlInjectionRuleCreateRisks(t *testing.T) { expectedImpact: types.HighImpact, }, "devops usage likely likelihood": { + assetType: types.Datastore, protocol: types.JdbcEncrypted, expectRiskCreated: true, isVulnerableToQueryInjection: true, @@ -117,6 +131,7 @@ func TestSqlNoSqlInjectionRuleCreateRisks(t *testing.T) { Id: "ta1", Title: "Test Technical Asset", OutOfScope: false, + Type: testCase.assetType, Technologies: types.TechnologyList{ { Name: "service-registry",