diff --git a/pkg/security/risks/builtin/sql_nosql_injection_rule.go b/pkg/security/risks/builtin/sql_nosql_injection_rule.go index 46331057..6d352301 100644 --- a/pkg/security/risks/builtin/sql_nosql_injection_rule.go +++ b/pkg/security/risks/builtin/sql_nosql_injection_rule.go @@ -44,12 +44,9 @@ func (r *SqlNoSqlInjectionRule) GenerateRisks(input *types.Model) ([]*types.Risk technicalAsset := input.TechnicalAssets[id] incomingFlows := input.IncomingTechnicalCommunicationLinksMappedByTargetId[technicalAsset.Id] for _, incomingFlow := range incomingFlows { - if input.TechnicalAssets[incomingFlow.SourceId].OutOfScope { - continue - } - potentialDatabaseAccessProtocol := incomingFlow.Protocol.IsPotentialDatabaseAccessProtocol(true) + potentialDatabaseAccessProtocol := incomingFlow.Protocol.IsPotentialDatabaseAccessProtocol() isVulnerableToQueryInjection := technicalAsset.Technologies.GetAttribute(types.IsVulnerableToQueryInjection) - potentialLaxDatabaseAccessProtocol := incomingFlow.Protocol.IsPotentialDatabaseAccessProtocol(false) + potentialLaxDatabaseAccessProtocol := incomingFlow.Protocol.IsPotentialLaxDatabaseAccessProtocol() if potentialDatabaseAccessProtocol && isVulnerableToQueryInjection || potentialLaxDatabaseAccessProtocol { risks = append(risks, r.createRisk(input, technicalAsset, incomingFlow)) diff --git a/pkg/security/risks/builtin/sql_nosql_injection_rule_test.go b/pkg/security/risks/builtin/sql_nosql_injection_rule_test.go index e72f3459..56cf7ff5 100644 --- a/pkg/security/risks/builtin/sql_nosql_injection_rule_test.go +++ b/pkg/security/risks/builtin/sql_nosql_injection_rule_test.go @@ -59,12 +59,18 @@ func TestSqlNoSqlInjectionRuleCreateRisks(t *testing.T) { expectRiskCreated: false, isVulnerableToQueryInjection: true, }, - // TODO: understand - // "not vulnerable to query injection": { - // protocol: types.JdbcEncrypted, - // expectRiskCreated: false, - // isVulnerableToQueryInjection: false, - // }, + "not vulnerable to query injection not lax": { + protocol: types.JdbcEncrypted, + expectRiskCreated: false, + isVulnerableToQueryInjection: false, + }, + "lax database always vulnerable to query injection": { + protocol: types.HTTP, + isVulnerableToQueryInjection: false, + expectRiskCreated: true, + expectedLikelihood: types.VeryLikely, + expectedImpact: types.MediumImpact, + }, "database protocol and vulnerable to query injection": { protocol: types.JdbcEncrypted, expectRiskCreated: true, diff --git a/pkg/security/types/protocol.go b/pkg/security/types/protocol.go index af3a3743..a1f03f22 100644 --- a/pkg/security/types/protocol.go +++ b/pkg/security/types/protocol.go @@ -7,8 +7,9 @@ package types import ( "encoding/json" "fmt" - "gopkg.in/yaml.v3" "strings" + + "gopkg.in/yaml.v3" ) type Protocol int @@ -195,14 +196,14 @@ func (what Protocol) IsEncrypted() bool { what == IiopEncrypted || what == JrmpEncrypted || what == SmbEncrypted || what == SmtpEncrypted || what == Pop3Encrypted || what == ImapEncrypted } -func (what Protocol) IsPotentialDatabaseAccessProtocol(includingLaxDatabaseProtocols bool) bool { - strictlyDatabaseOnlyProtocol := what == JdbcEncrypted || what == OdbcEncrypted || +func (what Protocol) IsPotentialDatabaseAccessProtocol() bool { + return what == JdbcEncrypted || what == OdbcEncrypted || what == NosqlAccessProtocolEncrypted || what == SqlAccessProtocolEncrypted || what == JDBC || what == ODBC || what == NosqlAccessProtocol || what == SqlAccessProtocol - if includingLaxDatabaseProtocols { - // include HTTP for REST-based NoSQL-DBs as well as unknown binary - return strictlyDatabaseOnlyProtocol || what == HTTPS || what == HTTP || what == BINARY || what == BinaryEncrypted - } - return strictlyDatabaseOnlyProtocol +} + +func (what Protocol) IsPotentialLaxDatabaseAccessProtocol() bool { + // include HTTP for REST-based NoSQL-DBs as well as unknown binary + return what == HTTPS || what == HTTP || what == BINARY || what == BinaryEncrypted } func (what Protocol) IsPotentialWebAccessProtocol() bool {