-
Notifications
You must be signed in to change notification settings - Fork 24
Add Java CWE-089 SQL injection audit query (partial flow, any source) #198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # Audit - SQL Injection (any source, partial flow) | ||
|
|
||
| Dynamically generated SQL queries built from unvalidated data can cause SQL injection attacks. | ||
| This audit query does not restrict itself to recognized remote/user-controlled sources (like | ||
| `RemoteFlowSource`). Instead, it performs a partial (backwards) taint-tracking exploration from | ||
| every known SQL injection sink, up to a limited number of steps, so that it can surface data that | ||
| flows into a SQL query even when the originating source is not (yet) modeled as user input. This | ||
| makes it useful for triaging new or unusual sources of tainted data that reach a SQL injection | ||
| sink. | ||
|
|
||
| Because any node is considered a potential source, this query is very low precision and is | ||
| intended for manual audit, not for inclusion in a default security suite. | ||
|
|
||
| ## Example | ||
|
|
||
| ```java | ||
| import java.sql.Connection; | ||
| import java.sql.Statement; | ||
|
|
||
| public class Example { | ||
| void run(Connection connection, String username) throws Exception { | ||
| Statement statement = connection.createStatement(); | ||
| // Building a query by concatenating an unsanitized value is a SQL injection sink, | ||
| // regardless of whether `username` is recognized as user input. | ||
| statement.executeQuery("SELECT * FROM users WHERE username = '" + username + "'"); | ||
| } | ||
| } | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /** | ||
| * @name SQL query built from user-controlled sources | ||
| * @description Building a SQL query from user-controlled sources is vulnerable to insertion of | ||
| * malicious SQL code by the user. This audit query reports partial flows (any | ||
| * source, not just recognized remote/user input) into a known SQL injection sink, | ||
| * which is useful for surfacing sources that are not yet modeled. | ||
| * @kind path-problem | ||
| * @problem.severity warning | ||
| * @security-severity 2.5 | ||
| * @sub-severity low | ||
| * @precision very-low | ||
| * @id githubsecuritylab/audit/sql-injection | ||
| * @tags security | ||
| * external/cwe/cwe-089 | ||
| * audit | ||
| */ | ||
|
|
||
| import java | ||
| import semmle.code.java.dataflow.DataFlow | ||
| import semmle.code.java.dataflow.TaintTracking | ||
| import semmle.code.java.security.QueryInjection | ||
|
|
||
| private module SqlInjectionAuditConfig implements DataFlow::ConfigSig { | ||
| predicate isSource(DataFlow::Node source) { any() } | ||
|
|
||
| predicate isSink(DataFlow::Node sink) { sink instanceof QueryInjectionSink } | ||
| } | ||
|
|
||
| private module SqlInjectionAuditFlow = TaintTracking::Global<SqlInjectionAuditConfig>; | ||
|
|
||
| int explorationLimit() { result = 10 } | ||
|
|
||
| private module SqlInjectionAuditPartialFlow = | ||
| SqlInjectionAuditFlow::FlowExplorationRev<explorationLimit/0>; | ||
|
|
||
| import SqlInjectionAuditPartialFlow::PartialPathGraph | ||
|
|
||
| from | ||
| SqlInjectionAuditPartialFlow::PartialPathNode source, | ||
| SqlInjectionAuditPartialFlow::PartialPathNode sink | ||
| where SqlInjectionAuditPartialFlow::partialFlow(source, sink, _) | ||
| select sink.getNode(), source, sink, "This SQL query depends on a $@.", source.getNode(), | ||
| "user-provided value" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| #select | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:6:37:6:51 | username | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:6:37:6:51 | username | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:20:8:59 | "SELECT * FROM users WHERE username = '" | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:20:8:59 | "SELECT * FROM users WHERE username = '" | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:20:8:59 | "SELECT * FROM users WHERE username = '" [Ext] | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:20:8:59 | "SELECT * FROM users WHERE username = '" | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... [Ext] | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:20:8:76 | ... + ... | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:20:8:76 | ... + ... | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:63:8:70 | username | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:63:8:70 | username | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:63:8:70 | username [Ext] | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:63:8:70 | username | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:74:8:76 | "'" | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:74:8:76 | "'" | user-provided value | | ||
| | SqlInjectionAuditTest.java:9:28:9:32 | query | SqlInjectionAuditTest.java:8:74:8:76 | "'" [Ext] | SqlInjectionAuditTest.java:9:28:9:32 | query | This SQL query depends on a $@. | SqlInjectionAuditTest.java:8:74:8:76 | "'" | user-provided value | | ||
| | SqlInjectionAuditTest.java:15:28:15:36 | hardcoded | SqlInjectionAuditTest.java:13:24:13:44 | "SELECT * FROM users" | SqlInjectionAuditTest.java:15:28:15:36 | hardcoded | This SQL query depends on a $@. | SqlInjectionAuditTest.java:13:24:13:44 | "SELECT * FROM users" | user-provided value | | ||
| edges | ||
| | SqlInjectionAuditTest.java:6:37:6:51 | username | SqlInjectionAuditTest.java:8:63:8:70 | username | | ||
| | SqlInjectionAuditTest.java:8:20:8:59 | "SELECT * FROM users WHERE username = '" | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | | ||
| | SqlInjectionAuditTest.java:8:20:8:59 | "SELECT * FROM users WHERE username = '" [Ext] | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | | ||
| | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | SqlInjectionAuditTest.java:8:20:8:76 | ... + ... | | ||
| | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... [Ext] | SqlInjectionAuditTest.java:8:20:8:76 | ... + ... | | ||
| | SqlInjectionAuditTest.java:8:20:8:76 | ... + ... | SqlInjectionAuditTest.java:9:28:9:32 | query | | ||
| | SqlInjectionAuditTest.java:8:63:8:70 | username | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | | ||
| | SqlInjectionAuditTest.java:8:63:8:70 | username [Ext] | SqlInjectionAuditTest.java:8:20:8:70 | ... + ... | | ||
| | SqlInjectionAuditTest.java:8:74:8:76 | "'" | SqlInjectionAuditTest.java:8:20:8:76 | ... + ... | | ||
| | SqlInjectionAuditTest.java:8:74:8:76 | "'" [Ext] | SqlInjectionAuditTest.java:8:20:8:76 | ... + ... | | ||
| | SqlInjectionAuditTest.java:13:24:13:44 | "SELECT * FROM users" | SqlInjectionAuditTest.java:15:28:15:36 | hardcoded | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| query: audit/CWE-089/SqlInjectionAudit.ql | ||
| postprocess: TestUtilities/PrettyPrintModels.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import java.sql.Connection; | ||
| import java.sql.SQLException; | ||
| import java.sql.Statement; | ||
|
|
||
| public class SqlInjectionAuditTest { | ||
| void test1(Connection connection, String username) throws SQLException { | ||
| Statement statement = connection.createStatement(); | ||
| String query = "SELECT * FROM users WHERE username = '" + username + "'"; | ||
| statement.executeQuery(query); | ||
|
felickz marked this conversation as resolved.
Dismissed
|
||
| } | ||
|
|
||
| void test2(Connection connection) throws SQLException { | ||
| String hardcoded = "SELECT * FROM users"; | ||
| Statement statement = connection.createStatement(); | ||
|
felickz marked this conversation as resolved.
Dismissed
|
||
| statement.executeQuery(hardcoded); | ||
|
felickz marked this conversation as resolved.
Dismissed
|
||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.