Skip to content

Potential fix for code scanning alert no. 46: Query built from user-controlled sources#6

Draft
davewichers wants to merge 1 commit intomainfrom
alert-autofix-46
Draft

Potential fix for code scanning alert no. 46: Query built from user-controlled sources#6
davewichers wants to merge 1 commit intomainfrom
alert-autofix-46

Conversation

@davewichers
Copy link
Member

Potential fix for https://github.com/aspectsecurity/TestCodeQL/security/code-scanning/46

In general, the fix is to stop concatenating user input into the SQL string and instead use parameter placeholders (?) for all variable parts, binding the actual values with PreparedStatement.setXxx methods. This ensures the database treats user input as data, not executable SQL.

For this specific file, we should change the SQL string on line 75 so that bar is no longer concatenated directly. Instead, define the query as "SELECT * from USERS where USERNAME=? and PASSWORD=?", and then bind both the username and password via PreparedStatement parameters. Since the existing code already binds the username at index 1 with "foo", we just need to add a second parameter binding for the password (bar) at index 2. No changes are needed to the way bar is computed; only its use in the query must be parameterized.

Concretely:

  • In Benchmark00839.doPost, replace the line building sql with a version that uses a second ? placeholder for the password.
  • After statement.setString(1, "foo");, add statement.setString(2, bar); so the user-controlled value is supplied safely.
  • No new imports or helper methods are needed; PreparedStatement is already used.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…ontrolled sources

Fix SQL injection in Benchmark00839.java:75

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant