Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-7g5f-wrx8-5ccf
* [GEOS-10839] Add JDBC Configuration parameter to disable SQL comments and pretty-printing

* [GEOS-10842] Escape user inputs in SQL queries

* [GEOS-10842] Applied PR feedback
  • Loading branch information
sikeoka committed Feb 13, 2023
1 parent ce73255 commit 145a8af
Show file tree
Hide file tree
Showing 12 changed files with 1,186 additions and 350 deletions.
Expand Up @@ -103,6 +103,7 @@
import org.geoserver.config.impl.CoverageAccessInfoImpl;
import org.geoserver.config.impl.GeoServerInfoImpl;
import org.geoserver.config.impl.JAIInfoImpl;
import org.geoserver.jdbcloader.JDBCLoaderProperties;
import org.geoserver.ows.util.OwsUtils;
import org.geoserver.platform.ExtensionPriority;
import org.geoserver.platform.resource.Resource;
Expand Down Expand Up @@ -160,6 +161,8 @@ public class ConfigDatabase implements ApplicationContextAware {

private Dialect dialect;

private JDBCLoaderProperties properties;

private DataSource dataSource;

private DbMappings dbMappings;
Expand Down Expand Up @@ -198,15 +201,19 @@ protected ConfigDatabase() {
//
}

public ConfigDatabase(final DataSource dataSource, final XStreamInfoSerialBinding binding) {
this(dataSource, binding, null);
public ConfigDatabase(
JDBCLoaderProperties properties,
DataSource dataSource,
XStreamInfoSerialBinding binding) {
this(properties, dataSource, binding, null);
}

public ConfigDatabase(
JDBCLoaderProperties properties,
final DataSource dataSource,
final XStreamInfoSerialBinding binding,
CacheProvider cacheProvider) {

this.properties = properties;
this.binding = binding;
this.template = new NamedParameterJdbcTemplate(dataSource);
// cannot use dataSource at this point due to spring context config hack
Expand All @@ -227,7 +234,7 @@ public ConfigDatabase(

private Dialect dialect() {
if (dialect == null) {
this.dialect = Dialect.detect(dataSource);
this.dialect = Dialect.detect(dataSource, properties.isDebugMode());
}
return dialect;
}
Expand Down Expand Up @@ -296,7 +303,7 @@ public <T extends CatalogInfo> int count(final Class<T> of, final Filter filter)

QueryBuilder<T> sqlBuilder = QueryBuilder.forCount(dialect, of, dbMappings).filter(filter);

final StringBuilder sql = sqlBuilder.build();
final String sql = sqlBuilder.build();
final Filter unsupportedFilter = sqlBuilder.getUnsupportedFilter();
final boolean fullySupported = Filter.INCLUDE.equals(unsupportedFilter);
if (LOGGER.isLoggable(Level.FINER)) {
Expand All @@ -309,7 +316,7 @@ public <T extends CatalogInfo> int count(final Class<T> of, final Filter filter)
final Map<String, Object> namedParameters = sqlBuilder.getNamedParameters();
logStatement(sql, namedParameters);

count = template.queryForObject(sql.toString(), namedParameters, Integer.class);
count = template.queryForObject(sql, namedParameters, Integer.class);
} else {
LOGGER.fine(
"Filter is not fully supported, doing scan of supported part to return the number of matches");
Expand Down Expand Up @@ -363,7 +370,7 @@ public <T extends Info> CloseableIterator<T> query(
.offset(offset)
.limit(limit)
.sortOrder(sortOrder);
final StringBuilder sql = sqlBuilder.build();
final String sql = sqlBuilder.build();

List<String> ids = null;

Expand Down Expand Up @@ -406,7 +413,7 @@ public <T extends Info> CloseableIterator<T> query(
// with rownum in the 2nd - queryForList will throw an exception
ids =
template.query(
sql.toString(),
sql,
namedParameters,
new RowMapper<String>() {
@Override
Expand Down Expand Up @@ -468,7 +475,7 @@ public <T extends Info> CloseableIterator<String> queryIds(

QueryBuilder<T> sqlBuilder = QueryBuilder.forIds(dialect, of, dbMappings).filter(filter);

final StringBuilder sql = sqlBuilder.build();
final String sql = sqlBuilder.build();
final Map<String, Object> namedParameters = sqlBuilder.getNamedParameters();
final Filter unsupportedFilter = sqlBuilder.getUnsupportedFilter();
final boolean fullySupported = Filter.INCLUDE.equals(unsupportedFilter);
Expand All @@ -485,7 +492,7 @@ public <T extends Info> CloseableIterator<String> queryIds(
// with rownum in the 2nd - queryForList will throw an exception
List<String> ids =
template.query(
sql.toString(),
sql,
namedParameters,
new RowMapper<String>() {
@Override
Expand Down
Expand Up @@ -8,12 +8,19 @@
import com.google.common.base.Joiner;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import javax.sql.DataSource;

public class Dialect {

public static Dialect detect(DataSource dataSource) {
// see https://github.com/hibernate/hibernate-orm/commit/59fede7acaaa1579b561407aefa582311f7ebe78
private static final Pattern ESCAPE_CLOSING_COMMENT_PATTERN = Pattern.compile("\\*/");
private static final Pattern ESCAPE_OPENING_COMMENT_PATTERN = Pattern.compile("/\\*");

private boolean debugMode;

public static Dialect detect(DataSource dataSource, boolean debugMode) {
Dialect dialect;
try {
Connection conn = dataSource.getConnection();
Expand All @@ -27,9 +34,46 @@ public static Dialect detect(DataSource dataSource) {
} catch (SQLException ex) {
throw new RuntimeException(ex);
}
dialect.setDebugMode(debugMode);
return dialect;
}

public boolean isDebugMode() {
return debugMode;
}

public void setDebugMode(boolean debugMode) {
this.debugMode = debugMode;
}

/** Escapes the contents of the SQL comment to prevent SQL injection. */
public String escapeComment(String comment) {
String escaped = ESCAPE_CLOSING_COMMENT_PATTERN.matcher(comment).replaceAll("*\\\\/");
return ESCAPE_OPENING_COMMENT_PATTERN.matcher(escaped).replaceAll("/\\\\*");
}

/** Appends the objects to the SQL in a comment if debug mode is enabled. */
public StringBuilder appendComment(StringBuilder sql, Object... objects) {
if (!debugMode) {
return sql;
}
sql.append(" /* ");
for (Object object : objects) {
sql.append(escapeComment(String.valueOf(object)));
}
return sql.append(" */\n");
}

/** Appends the objects to the SQL in an comment if debug mode is enabled. */
public StringBuilder appendComment(Object sql, Object... objects) {
return appendComment((StringBuilder) sql, objects);
}

/** Appends one of the strings to the SQL depending on whether debug mode is enabled. */
public StringBuilder appendIfDebug(StringBuilder sql, String ifEnabled, String ifDisabled) {
return sql.append(debugMode ? ifEnabled : ifDisabled);
}

public void applyOffsetLimit(
StringBuilder sql, @Nullable Integer offset, @Nullable Integer limit) {
// some db's require limit to be present of offset is
Expand Down

0 comments on commit 145a8af

Please sign in to comment.