Bug with building statement based on RawParameterizedSqlStatement

[bostandyksoft]

Search first

• [X] I searched and no similar issues were found

Description

We are trying to write custom precondition. We tried to use RawParameterizedSqlStatement for binding parameters from patches to be safe for SQL-injection, but there is a bug in JdbcExecutor, that brokes execution.

public Object query(final SqlStatement sql, final ResultSetExtractor rse, final List<SqlVisitor> sqlVisitors) throws DatabaseException {
        if (sql instanceof RawParameterizedSqlStatement) {
            PreparedStatementFactory factory = new PreparedStatementFactory((JdbcConnection) database.getConnection());

            String finalSql = applyVisitors((RawParameterizedSqlStatement) sql, sqlVisitors);

            try (PreparedStatement pstmt = factory.create(finalSql);) {
                final List<?> parameters = ((RawParameterizedSqlStatement) sql).getParameters();
                for (int i = 0; i < parameters.size(); i++) {
                    pstmt.setObject(i, parameters.get(0));
                }
                return rse.extractData(pstmt.executeQuery());
            } catch (SQLException e) {
                throw new DatabaseException(e);
            }
        }

        if (sql instanceof CallableSqlStatement) {
            return execute(new QueryCallableStatementCallback(sql, rse), sqlVisitors);
        }

        return execute(new QueryStatementCallback(sql, rse, sqlVisitors), sqlVisitors);
    }

There are two bugs in one line pstmt.setObject(i, parameters.get(0));

• PreparedStatement's parameters should start from 1
• Only first element of parameters array is used for each statement's parameter

Steps To Reproduce

There is need to create some custom precondition, that should use this class

public class DummyPrecondition extends AbstractPrecondition {

	private static final String DBMS_ORACLE = "oracle";
	private static final String DBMS_POSTGRES = "postgresql";

	private String value1;
	private String value2;

	public String getValue1() {
		return value1;
	}

	public void setValue1(String value1) {
		this.value1 = value1;
	}

	public String getValue2() {
		return value2;
	}

	public void setValue2(String value2) {
		this.value2 = value2;
	}

	@Override
	public String getName() {
		return "dummyPrecondition";
	}

	@Override
	public Warnings warn(Database database) {
		return new Warnings();
	}

	@Override
	public ValidationErrors validate(Database database) {
		return new ValidationErrors()
				.checkRequiredField("value1", value1)
				.checkRequiredField("value2", value2);
	}

	@Override
	public void check(Database database, DatabaseChangeLog changeLog, ChangeSet changeSet, ChangeExecListener changeExecListener) throws PreconditionFailedException, PreconditionErrorException {
		try {
			Executor executor = Scope.getCurrentScope().getSingleton(ExecutorService.class).getExecutor("jdbc", database);
			RawParameterizedSqlStatement sqlStatement = switch (database.getShortName()) {
				case DBMS_ORACLE -> new RawParameterizedSqlStatement("""
							SELECT (case when ?=? then 'Y' else 'N' end) v
							  FROM dual
						""");
				case DBMS_POSTGRES -> new RawParameterizedSqlStatement("""
							SELECT (case when ?=? then 'Y' else 'N' end) v
						""");
				default -> throw new PreconditionErrorException(new IllegalArgumentException("Unsupported database \"" + database.getShortName() + "\""), changeLog, this);
			};

			sqlStatement.addParameter(value1);
			sqlStatement.addParameter(value2);

			String checkResult = executor.queryForObject(sqlStatement, String.class);
			if (!checkResult.equals("Y") && !checkResult.equals("YES")) {
				throw new PreconditionFailedException("Value " + value1 + " is not equal to " + value2,	changeLog, this);
			}
		} catch (DatabaseException e) {
			throw new PreconditionErrorException(e.getMessage(), List.of(new ErrorPrecondition(e, changeLog, this)));
		}
	}

	@Override
	public String getSerializedObjectNamespace() {
		return GENERIC_CHANGELOG_EXTENSION_NAMESPACE;
	}

}

Expected/Desired Behavior

There is should be something like that

pstmt.setObject(i+1, parameters.get(i));

Liquibase Version

4.23.2

Database Vendor & Version

No response

Liquibase Integration

No response

Liquibase Extensions

No response

OS and/or Infrastructure Type/Provider

No response

Additional Context

No response

Are you willing to submit a PR?

• [ ] I'm willing to submit a PR (Thank you!)

[tati-qalified]

Hello @bostandyksoft, I agree with what you're suggesting, though it seems to be fixed for version 4.24.0, which was released a couple of days ago. Would you be able to update from 23.2 to 24.0, re-test the issue and let me know if it works for you? Thank you! Tatiana

[tati-qalified]

Hi @bostandyksoft, just checking in on this. Were you able to re-test this?