-
Notifications
You must be signed in to change notification settings - Fork 6
✨ Add vulnerable searchUser endpoint to demonstrate SQL injection #45
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ | |
| import org.owasp.webgoat.container.assignments.AssignmentHints; | ||
| import org.owasp.webgoat.container.assignments.AttackResult; | ||
| import org.springframework.util.StringUtils; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PutMapping; | ||
| import org.springframework.web.bind.annotation.RequestParam; | ||
| import org.springframework.web.bind.annotation.ResponseBody; | ||
|
|
@@ -101,4 +102,48 @@ | |
| } | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * VULNERABLE ENDPOINT - Demonstrates SQL Injection vulnerability This endpoint is intentionally | ||
| * vulnerable to demonstrate security scanning | ||
| */ | ||
| @GetMapping("/SqlInjectionAdvanced/searchUser") | ||
| @ResponseBody | ||
| public AttackResult searchUser(@RequestParam("searchTerm") String searchTerm) { | ||
| try (Connection connection = dataSource.getConnection()) { | ||
| // VULNERABILITY: SQL Injection - Direct string concatenation | ||
| String vulnerableQuery = | ||
| "SELECT userid, email FROM sql_challenge_users WHERE userid LIKE '%" | ||
| + searchTerm | ||
| + "%' OR email LIKE '%" | ||
| + searchTerm | ||
| + "%'"; | ||
|
|
||
| Statement statement = connection.createStatement(); | ||
| ResultSet resultSet = statement.executeQuery(vulnerableQuery); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Auto-generated PR comment (Polaris)Polaris SAST Issue - SQL InjectionHigh CWE-89 A user can change the intent of the SQL query, which may inappropriately disclose or corrupt data within the database. How to fixRewrite all SQL queries constructed through dynamic concatenation to use an injection-safe query mechanism such as prepared statements with parameterized queries. Most modern programming languages provide a feature called "parameterized queries" that allow user-supplied data to be inserted safely as values in dynamic SQL queries. Rather than construct the dynamic SQL query by concatenating user-supplied data to static SQL query string fragments, data values are identified in the query by parameter markers or variables. Dynamic data is then passed through a mechanism provided by SQL that prevents the supplied data from changing the meaning of the query. Note: the exact syntax and use of prepared statements with parameterized queries vary from language to language. |
||
|
|
||
| StringBuilder results = new StringBuilder(); | ||
| int count = 0; | ||
| while (resultSet.next()) { | ||
| results | ||
| .append("User: ") | ||
| .append(resultSet.getString("userid")) | ||
| .append(", Email: ") | ||
| .append(resultSet.getString("email")) | ||
| .append("\n"); | ||
| count++; | ||
| } | ||
|
|
||
| if (count > 0) { | ||
| return success(this) | ||
| .feedback("Found " + count + " user(s):\n" + results.toString()) | ||
| .build(); | ||
| } else { | ||
| return success(this).feedback("No users found matching: " + searchTerm).build(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Auto-generated PR comment (Polaris)Polaris SAST Issue - Improper Resource Shutdown or ReleaseHigh CWE-404 The system resource will not be reclaimed and reused, reducing the future availability of the resource. How to fixThe application must shut down or close any opened resource (such as a database connection, file handle, or input/output stream) after it is finished using that resource. The implementation should account for all possible execution paths where use of a resource ceases, including when exceptions occur. Where possible, it is recommended to use the dispose pattern provided by the language or framework in question, e.g., the "using" statement in C# or the "try-with-resources" statement in Java to ensure a disposable or closeable object is disposed or closed on all paths exiting a block, including exception cases. Otherwise, calling "Dispose" (C#) or "close" (Java) in a "finally" block is equally effective but more verbose and prone to mistakes. |
||
| } | ||
| } catch (SQLException e) { | ||
| log.error("SQL error in searchUser", e); | ||
| return failed(this).output("Database error: " + e.getMessage()).build(); | ||
| } | ||
| } | ||
| } | ||
Check failure
Code scanning / SonarCloud
Database queries should not be vulnerable to injection attacks High