Vulnerability fix (powered by Mobb Autofixer)#142
Conversation
![semgrep-code-antonychiu2[bot]](https://avatars.githubusercontent.com/u/5158535?s=60&v=4){kind=small}
| log(connection, query); | ||
| ResultSet results = statement.executeQuery(query); | ||
| statement.setString(1, auth_tan); | ||
| ResultSet results = statement.executeQuery(); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L67 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 67] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L65 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 65] query</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L72 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 72] statement</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L77 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 77] statement.executeQuery()</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| ResultSet results = statement.executeQuery(); | |
| String query = | |
| "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; | |
| try (Connection connection = dataSource.getConnection()) { | |
| try { | |
| PreparedStatement statement = | |
| connection.prepareStatement( | |
| query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); | |
| log(connection, query); | |
| statement.setString(1, name); | |
| statement.setString(2, auth_tan); // Securely parameterize auth_tan | |
| ResultSet results = statement.executeQuery(); |
View step-by-step instructions
- Change the SQL query construction to remove direct concatenation of the
namevariable into the statement. Instead, use a parameter placeholder (?) for both input values. - Update the query assignment to:
String query = "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; - After preparing the statement, bind the
nameparameter to index 1 withstatement.setString(1, name). - Bind the
auth_tanparameter to index 2 withstatement.setString(2, auth_tan). - Remove any previous code that concatenates user inputs into the SQL statement.
This prevents SQL injection by making sure user inputs are not assembled directly into the SQL command string. Prepared statements ensure proper escaping of parameters.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by formatted-sql-string.
You can view more details about this finding in the Semgrep AppSec Platform.
| "SELECT * FROM employees WHERE last_name = '" | ||
| + name | ||
| + "' AND auth_tan = '" | ||
| + auth_tan | ||
| + "'"; | ||
| + "' AND auth_tan = ?"; |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
User data flows into this manually-constructed SQL string. User data can be safely inserted into SQL strings using prepared statements or an object-relational mapper (ORM). Manually-constructed SQL strings is a possible indicator of SQL injection, which could let an attacker steal or manipulate data from the database. Instead, use prepared statements (connection.PreparedStatement) or a safe library.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L61 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 61] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L61 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 61] name</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L62 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 62] injectableQueryIntegrity</a>"]
v4["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L65 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 65] name</a>"]
end
v2 --> v3
v3 --> v4
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L68 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 68] "SELECT * FROM employees WHERE last_name = '"<br> + name<br> + "' AND auth_tan = ?"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| "SELECT * FROM employees WHERE last_name = '" | |
| + name | |
| + "' AND auth_tan = '" | |
| + auth_tan | |
| + "'"; | |
| + "' AND auth_tan = ?"; | |
| // Use a parameterized query to prevent SQL injection | |
| String query = "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; |
View step-by-step instructions
- Change the SQL query construction so that user-provided data (
name) is not directly concatenated into the query string. - Use a parameterized query with placeholders instead of concatenation. For example, rewrite the query as:
"SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?" - Create the prepared statement with this parameterized query:
PreparedStatement statement = connection.prepareStatement(query, ...) - Set the value for the first parameter (last_name) using
statement.setString(1, name) - Set the value for the second parameter (auth_tan) using
statement.setString(2, auth_tan) - Execute the query as usual.
This change will ensure that the SQL driver safely handles the user-provided data and prevents SQL injection vulnerabilities.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by tainted-sql-string.
You can view more details about this finding in the Semgrep AppSec Platform.
| try (Connection connection = dataSource.getConnection()) { | ||
| try { | ||
| Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); | ||
| PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Untrusted input might be used to build a database query, which can lead to a SQL injection vulnerability. An attacker can execute malicious SQL statements and gain unauthorized access to sensitive data, modify, delete data, or execute arbitrary system commands. To prevent this vulnerability, use prepared statements that do not concatenate user-controllable strings and use parameterized queries where SQL commands and user data are strictly separated. Also, consider using an object-relational (ORM) framework to operate with safer abstractions. To build SQL queries safely in Java, it is possible to adopt prepared statements by using the java.sql.PreparedStatement class with bind variables.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L61 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 61] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L61 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 61] name</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L62 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 62] injectableQueryIntegrity</a>"]
v4["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L65 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 65] name</a>"]
v5["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L67 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 67] query</a>"]
end
v2 --> v3
v3 --> v4
v4 --> v5
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L73 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 73] query</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); | |
| // Fix: Parameterize both 'name' and 'auth_tan' to prevent SQL injection. | |
| String query = "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; | |
| PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); | |
| SqlInjectionLesson8.log(connection, query); | |
| statement.setString(1, name); | |
| statement.setString(2, auth_tan); | |
| ResultSet results = statement.executeQuery(); |
View step-by-step instructions
- Update your SQL query string so that all user-controllable values are parameterized using
?placeholders, rather than directly concatenating inputs likenameinto the query. For example, instead of"'SELECT * FROM employees WHERE last_name = '" + name + "' AND auth_tan = ?'", use"SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?". - Pass both user input values to the prepared statement using
statement.setString. For example:statement.setString(1, name);statement.setString(2, auth_tan);
- Ensure the query preparation and execution looks like this:
PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE);SqlInjectionLesson8.log(connection, query);statement.setString(1, name);statement.setString(2, auth_tan);ResultSet results = statement.executeQuery();
By parameterizing all user input in your SQL queries, you prevent attackers from injecting malicious statements, since the database treats input values as data, not as a part of the query syntax.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by java-sql-sqli.
You can view more details about this finding in the Semgrep AppSec Platform.
| "SELECT * FROM employees WHERE last_name = '" | ||
| + name | ||
| + "' AND auth_tan = '" | ||
| + auth_tan | ||
| + "'"; | ||
| + "' AND auth_tan = ?"; |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
User data flows into this manually-constructed SQL string. User data can be safely inserted into SQL strings using prepared statements or an object-relational mapper (ORM). Manually-constructed SQL strings is a possible indicator of SQL injection, which could let an attacker steal or manipulate data from the database. Instead, use prepared statements (connection.PreparedStatement) or a safe library.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L59 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 59] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L59 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 59] name</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L60 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 60] injectableQueryConfidentiality</a>"]
v4["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L63 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 63] name</a>"]
end
v2 --> v3
v3 --> v4
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L66 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 66] "SELECT * FROM employees WHERE last_name = '"<br> + name<br> + "' AND auth_tan = ?"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| "SELECT * FROM employees WHERE last_name = '" | |
| + name | |
| + "' AND auth_tan = '" | |
| + auth_tan | |
| + "'"; | |
| + "' AND auth_tan = ?"; | |
| // Use parameter placeholders for both 'name' and 'auth_tan' | |
| String query = | |
| "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; |
View step-by-step instructions
- Change the way you construct the
querystring to use parameter placeholders (?) for all user input.
Instead of includingnamedirectly in the SQL string, use:"SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?" - Remove the string concatenation with
namefrom the query definition. - Set the parameters for both
nameandauth_tanin thePreparedStatementusingstatement.setString(1, name)andstatement.setString(2, auth_tan). - Adjust any references to parameter indexes, since there will now be two parameters to set.
Prepared statements safely handle user input by ensuring input values are never parsed as SQL.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by tainted-sql-string.
You can view more details about this finding in the Semgrep AppSec Platform.
| try (Connection connection = dataSource.getConnection()) { | ||
| try { | ||
| Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); | ||
| PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Untrusted input might be used to build a database query, which can lead to a SQL injection vulnerability. An attacker can execute malicious SQL statements and gain unauthorized access to sensitive data, modify, delete data, or execute arbitrary system commands. To prevent this vulnerability, use prepared statements that do not concatenate user-controllable strings and use parameterized queries where SQL commands and user data are strictly separated. Also, consider using an object-relational (ORM) framework to operate with safer abstractions. To build SQL queries safely in Java, it is possible to adopt prepared statements by using the java.sql.PreparedStatement class with bind variables.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L61 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 61] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L61 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 61] name</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L62 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 62] injectableQueryIntegrity</a>"]
v4["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L65 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 65] name</a>"]
v5["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L67 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 67] query</a>"]
end
v2 --> v3
v3 --> v4
v4 --> v5
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L73 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 73] query</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); | |
| String query = | |
| "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; | |
| try (Connection connection = dataSource.getConnection()) { | |
| try { | |
| PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); | |
| SqlInjectionLesson8.log(connection, query); | |
| statement.setString(1, name); // Set the 'name' parameter | |
| statement.setString(2, auth_tan); // Set the 'auth_tan' parameter | |
| ResultSet results = statement.executeQuery(); | |
| var test = results.getRow() != 0; | |
| if (results.getStatement() != null) { | |
| if (results.first()) { | |
| output.append(SqlInjectionLesson8.generateTable(results)); | |
| } else { | |
| // no results | |
| return failed(this).feedback("sql-injection.8.no.results").build(); | |
| } | |
| } | |
| } catch (SQLException e) { | |
| System.err.println(e.getMessage()); | |
| return failed(this) | |
| .output("<br><span class='feedback-negative'>" + e.getMessage() + "</span>") | |
| .build(); | |
| } | |
| return checkSalaryRanking(connection, output); | |
| } catch (Exception e) { | |
| System.err.println(e.getMessage()); | |
| return failed(this) | |
| .output("<br><span class='feedback-negative'>" + e.getMessage() + "</span>") | |
| .build(); | |
| } |
View step-by-step instructions
- Change the SQL query in
injectableQueryIntegrityso that bothnameandauth_tanuse parameter placeholders (?) rather than directly concatenatingnameinto the query string. The query should look like:"SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?". - When creating the
PreparedStatement, set both parameters usingstatement.setString(1, name);andstatement.setString(2, auth_tan);, making sure to set them in the correct order as they appear in the query. - Remove the string concatenation of untrusted data in the SQL query string.
This change ensures user input is never directly inserted into the SQL query, which helps prevent SQL injection attacks.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by formatted-sql-string-deepsemgrep.
You can view more details about this finding in the Semgrep AppSec Platform.
| ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); | ||
| PreparedStatement statement = | ||
| connection.prepareStatement( | ||
| query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Untrusted input might be used to build a database query, which can lead to a SQL injection vulnerability. An attacker can execute malicious SQL statements and gain unauthorized access to sensitive data, modify, delete data, or execute arbitrary system commands. To prevent this vulnerability, use prepared statements that do not concatenate user-controllable strings and use parameterized queries where SQL commands and user data are strictly separated. Also, consider using an object-relational (ORM) framework to operate with safer abstractions. To build SQL queries safely in Java, it is possible to adopt prepared statements by using the java.sql.PreparedStatement class with bind variables.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L59 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 59] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L59 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 59] name</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L60 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 60] injectableQueryConfidentiality</a>"]
v4["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L63 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 63] name</a>"]
v5["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L65 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 65] query</a>"]
end
v2 --> v3
v3 --> v4
v4 --> v5
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L74 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 74] query</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); | |
| // Change the query to use parameter placeholders for both 'name' and 'auth_tan' | |
| String query = | |
| "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; | |
| try (Connection connection = dataSource.getConnection()) { | |
| try { | |
| PreparedStatement statement = | |
| connection.prepareStatement( | |
| query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); | |
| // Set both user inputs via safe parameter binding | |
| statement.setString(1, name); | |
| statement.setString(2, auth_tan); | |
| log(connection, query); | |
| ResultSet results = statement.executeQuery(); | |
| if (results.getStatement() != null) { | |
| if (results.first()) { | |
| output.append(generateTable(results)); | |
| results.last(); | |
| if (results.getRow() > 1) { | |
| // more than one record, the user succeeded | |
| return success(this) | |
| .feedback("sql-injection.8.success") | |
| .output(output.toString()) | |
| .build(); | |
| } else { | |
| // only one record | |
| return failed(this).feedback("sql-injection.8.one").output(output.toString()).build(); | |
| } | |
| } else { | |
| // no results | |
| return failed(this).feedback("sql-injection.8.no.results").build(); | |
| } | |
| } else { | |
| return failed(this).build(); | |
| } | |
| } catch (SQLException e) { | |
| return failed(this) | |
| .output("<br><span class='feedback-negative'>" + e.getMessage() + "</span>") |
View step-by-step instructions
- Update the SQL query so that both user inputs (
nameandauth_tan) are included as bind variables (i.e., placeholders), instead of concatenatingnamedirectly into the query string. Change the query construction line to:
String query = "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; - In the code that prepares and sets query parameters for the
PreparedStatement, set both inputs as parameters, not onlyauth_tan. After you prepare the statement, add:
statement.setString(1, name);
statement.setString(2, auth_tan);
(Adjust indices if you have additional parameters.) - Remove any direct concatenation of user data into an SQL statement string.
Prepared statements keep user-supplied input separate from the SQL command, helping prevent SQL injection attacks.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by java-sql-sqli.
You can view more details about this finding in the Semgrep AppSec Platform.
| ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); | ||
| PreparedStatement statement = | ||
| connection.prepareStatement( | ||
| query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Untrusted input might be used to build a database query, which can lead to a SQL injection vulnerability. An attacker can execute malicious SQL statements and gain unauthorized access to sensitive data, modify, delete data, or execute arbitrary system commands. To prevent this vulnerability, use prepared statements that do not concatenate user-controllable strings and use parameterized queries where SQL commands and user data are strictly separated. Also, consider using an object-relational (ORM) framework to operate with safer abstractions. To build SQL queries safely in Java, it is possible to adopt prepared statements by using the java.sql.PreparedStatement class with bind variables.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L59 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 59] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L59 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 59] name</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L60 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 60] injectableQueryConfidentiality</a>"]
v4["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L63 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 63] name</a>"]
v5["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L65 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 65] query</a>"]
end
v2 --> v3
v3 --> v4
v4 --> v5
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java#L74 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 74] query</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); | |
| String query = | |
| "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; | |
| try (Connection connection = dataSource.getConnection()) { | |
| try { | |
| PreparedStatement statement = | |
| connection.prepareStatement( | |
| query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); | |
| log(connection, query); | |
| // Set parameters for last_name and auth_tan securely using parameterized query | |
| statement.setString(1, name); | |
| statement.setString(2, auth_tan); | |
| ResultSet results = statement.executeQuery(); |
View step-by-step instructions
- Change the SQL query definition so that all user input values are replaced with placeholders (
?) rather than being directly concatenated into the string. Specifically, update the line building thequeryvariable to use a?for thenameparameter:
String query = "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; - When using the
PreparedStatement, set values for both placeholders with the provided user inputs. Update the lines after creating thePreparedStatementso thatstatement.setString(1, name);andstatement.setString(2, auth_tan);are used in that order. - Execute the query as normal.
By using prepared statements and bind variables for all user input, you prevent SQL injection because the input cannot alter the SQL query structure.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by formatted-sql-string-deepsemgrep.
You can view more details about this finding in the Semgrep AppSec Platform.
| SqlInjectionLesson8.log(connection, query); | ||
| ResultSet results = statement.executeQuery(query); | ||
| statement.setString(1, auth_tan); | ||
| ResultSet results = statement.executeQuery(); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L69 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 69] name</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L67 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 67] query</a>"]
v3["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L73 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 73] statement</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/antonychiu2/webgoat/blob/86e1e273e8b381856b6f818c6e4d4c3ab59e7f59/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java#L76 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 76] statement.executeQuery()</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| ResultSet results = statement.executeQuery(); | |
| protected AttackResult injectableQueryIntegrity(String name, String auth_tan) { | |
| StringBuilder output = new StringBuilder(); | |
| // Update the query to use parameter placeholders instead of direct concatenation | |
| String query = | |
| "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; | |
| try (Connection connection = dataSource.getConnection()) { | |
| try { | |
| PreparedStatement statement = connection.prepareStatement(query, TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); | |
| SqlInjectionLesson8.log(connection, query); | |
| statement.setString(1, name); | |
| statement.setString(2, auth_tan); // Bind the auth_tan parameter safely | |
| ResultSet results = statement.executeQuery(); | |
| var test = results.getRow() != 0; | |
| if (results.getStatement() != null) { | |
| if (results.first()) { | |
| output.append(SqlInjectionLesson8.generateTable(results)); | |
| } else { | |
| // no results | |
| return failed(this).feedback("sql-injection.8.no.results").build(); | |
| } | |
| } | |
| } catch (SQLException e) { | |
| System.err.println(e.getMessage()); | |
| return failed(this) | |
| .output("<br><span class='feedback-negative'>" + e.getMessage() + "</span>") | |
| .build(); | |
| } | |
| return checkSalaryRanking(connection, output); | |
| } catch (Exception e) { | |
| System.err.println(e.getMessage()); | |
| return failed(this) | |
| .output("<br><span class='feedback-negative'>" + e.getMessage() + "</span>") | |
| .build(); | |
| } | |
| } |
View step-by-step instructions
- Change the SQL query string to use a parameter placeholder (
?) forauth_taninstead of concatenating it directly. For example, update the query to:"SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?" - Replace concatenation of
auth_tanin the query string with the?placeholder. - Add
statement.setString(2, auth_tan);after setting the first parameter (name) to bind the value ofauth_tansafely to the query. - Remove any direct concatenation of user input into the SQL query.
By updating the query to use parameter placeholders and binding all user input values, you ensure inputs are treated as data, not executable SQL code. This prevents SQL injection vulnerabilities.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by formatted-sql-string.
You can view more details about this finding in the Semgrep AppSec Platform.
1 participant
This change fixes 6 issues reported by Codeql.
SQL Injection (6)
Issue description
SQL Injection allows attackers to execute malicious SQL queries by manipulating input data. This can result in unauthorized access to sensitive data, data manipulation, or even complete database compromise.
Fix instructions
Use parameterized queries or prepared statements to sanitize user input and prevent manipulation of the SQL query.
Additional info and fix customization on Mobb platform
SQL_Injection fix 1 SQL_Injection fix 2 SQL_Injection fix 3 SQL_Injection fix 4 SQL_Injection fix 5 SQL_Injection fix 6