CODEWITHSUNDEEP
javascriptweb-applicationsprepared-statementsql-injection
RobertS
RobertS

Reputation: 3

Prepared Statement with SQL Query issue

The code displayed below is used to search through job listings, the prepared statement was added in to prevent SQL injections which it is vulnerable too. The query statement search worked before adding the prepared statement, however, now returns nothing. Can anyone see what I'm doing wrong? Thanks. The previous code looked like this:

try (Connection con = DriverManager.getConnection("jdbc:mysql://localhost:3306/SQL_DB", "root", "root"); Statement stmt = con.createStatement()) { rs = stmt.executeQuery(queryStatement);

The code now looks like this:

public List<jobs> searchjobs(@RequestParam String query) {
    log.debug("REST request to search jobs for query {}", query);


    String queryStatement = "SELECT * FROM jobs WHERE description like '%" + query + "%'";
    log.info("Final SQL query {}", queryStatement);

    ResultSet rs = null;

    try (Connection con = DriverManager.getConnection("jdbc:mysql://localhost:3306/SQL_DB", "root", "root");
        PreparedStatement stmt = con.prepareStatement(queryStatement)) {
        stmt.setString(1, query);
        rs = stmt.executeQuery(queryStatement);
        return extractjobs(rs);
    } catch (SQLException ex) {
        log.error(ex.getMessage(), ex);
        return new ArrayList();
    } finally {
        try {
            if (rs != null) {
                rs.close();
            }
        } catch (SQLException ex) {
            log.error(ex.getMessage(), ex);
        }
    }
}

Upvotes: 0

Views: 171

Answers (2)

Your Common Sense
Your Common Sense

Reputation: 157889

You missed the essence of a prepared statement. When using it, you are substituting all variables with placeholders like this

String queryStatement = "SELECT * FROM jobs WHERE description like ?";

and then you can bind the query variable like you did. Just add those percent characters to it first

stmt.setString(1, "%" + query + "%");

Upvotes: 1

Bill Karwin
Bill Karwin

Reputation: 562418

You might have read the common advice "use prepared statements to protect against SQL injection." But this advice is only half of the truth.

The full truth is that you should use query parameters to separate untrusted content from your query. It just happens that using query parameters requires you to use prepared statements, so you can separate the prepare step from the execute step, and bind parameters to your prepared query in between these two steps.

This helps protect against SQL injection because the prepare is when your query is parsed. If you keep untrusted content separate from the query until after it's parsed, then the untrusted content cannot introduce SQL injection.

Your query has no parameter placeholders. You're concatenating your untrusted content into the statement just like before, which means it still has an SQL injection vulnerability.

String queryStatement = "SELECT * FROM jobs WHERE description like '%" + query + "%'";

Should be:

String queryStatement = "SELECT * FROM jobs WHERE description like ?";

Since your query has no parameter placeholders, it should be an error when you attempt to bind to it:

    stmt.setString(1, query);

Does this throw an exception? You seem to log it, so check your log.

Upvotes: 1

Related Questions