Merge pull request #74 from Threagile/fix-sql-injection-rule

Fix sql injection rule
Threagile · Jun 5, 2024 · 1099f90 · 1099f90
2 parents a137a7f + ead1d13
commit 1099f90
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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 {