Threagile / threagile

Agile Threat Modeling Toolkit
https://threagile.io
MIT License
577 stars 126 forks source link

Fix sql injection rule #74

Closed ezavgorodniy closed 4 weeks ago

ezavgorodniy commented 4 weeks ago

From slack conversation with @joreiche:

Yevhen Zavhorodnii 11:25 AM I believe it should be like here: https://github.com/Threagile/threagile/pull/74 So: Lax database connection is only HTTP/HTTPS/Binary/BinaryEncrypted For lax database connection we're only creating a risk even when technology is not vulnerable to query injection For normal database connection we're only creating a risk when technology is vulnerable to query injection

Joerg Reichelt 5:50 PM you can do a query injection over any protocol - even encrypted.

Joerg Reichelt 5:56 PM what does lax mean in this context? I am assuming that it means there isn't the same strict syntax checking as with sql but that may or may not be true. in any case, if we assume that non-strict db protocols are more vulnerable to query injection then the logic is correct because it would mean: query injection is a potential issue if either you have a strict db protocol and that protocol is vulnerable to query injection, or you have a non-strict db protocol (in which case query injection vulnerability is implied)

Joerg Reichelt 5:58 PM however, I would suggest to make the evaluation precedence explicit by saying: if (potentialDatabaseAccessProtocol && isVulnerableToQueryInjection) || potentialLaxDatabaseAccessProtocol { add risk }

Sounds like LGTM from him for me