Skip to content

Commit

Permalink
XWIKI-22726: Allow customizing the validation of HQL queries through …
Browse files Browse the repository at this point in the history
…configuration (#3747) (#3763)

* introduce an (internal) extensible validation system HQL queries
* introduce a new validator based on configurable regex of safe/unsafe regex patterns

(cherry picked from commit 620256c)
  • Loading branch information
github-actions[bot] authored Dec 13, 2024
1 parent a2b4898 commit 0b26038
Show file tree
Hide file tree
Showing 11 changed files with 591 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xwiki.query.Query;
import org.xwiki.query.WrappingQuery;

import net.sf.jsqlparser.JSQLParserException;
import net.sf.jsqlparser.expression.Expression;
Expand Down Expand Up @@ -343,4 +345,50 @@ public static String replaceLegacyQueryParameters(String queryString)
convertedString = builder.toString();
}
}

/**
* @param statement a potentially short form statement to complete
* @return a complete version of the input {@link Query} (or the {@link Query} as is if it's already complete)
* @since 17.0.0RC1
* @since 16.10.2
* @since 15.10.16
* @since 16.4.6
*/
public static String toCompleteStatement(String statement)
{
String completeStatement = statement;

if (StringUtils.isEmpty(statement) || isShortFormStatement(statement)) {
completeStatement = "select doc.fullName from XWikiDocument doc " + statement.trim();
}

return completeStatement;
}

/**
* @param query a potentially short form query to complete
* @return a complete version of the input {@link Query} (or the {@link Query} as is if it's already complete)
* @since 17.0.0RC1
* @since 16.10.2
* @since 15.10.16
* @since 16.4.6
*/
public static Query toCompleteQuery(Query query)
{
Query completeQuery = query;

String completeStatement = toCompleteStatement(query.getStatement());
if (completeStatement != query.getStatement()) {
completeQuery = new WrappingQuery(query)
{
@Override
public String getStatement()
{
return completeStatement;
}
};
}

return completeQuery;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@
import javax.inject.Singleton;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.hibernate.Session;
import org.hibernate.cfg.Configuration;
import org.hibernate.engine.spi.NamedQueryDefinition;
import org.hibernate.engine.spi.NamedSQLQueryDefinition;
import org.hibernate.query.NativeQuery;
import org.slf4j.Logger;
import org.xwiki.component.annotation.Component;
import org.xwiki.component.manager.ComponentLookupException;
import org.xwiki.component.manager.ComponentManager;
Expand All @@ -50,6 +52,7 @@
import org.xwiki.query.QueryParameter;
import org.xwiki.query.SecureQuery;
import org.xwiki.query.WrappingQuery;
import org.xwiki.query.hql.internal.HQLStatementValidator;
import org.xwiki.security.authorization.ContextualAuthorizationManager;
import org.xwiki.security.authorization.Right;

Expand All @@ -58,13 +61,15 @@
import com.xpn.xwiki.internal.store.hibernate.query.HqlQueryUtils;
import com.xpn.xwiki.store.XWikiHibernateStore;
import com.xpn.xwiki.util.Util;
import com.xpn.xwiki.web.Utils;

/**
* QueryExecutor implementation for Hibernate Store.
* {@link QueryExecutor} implementation for Hibernate Store.
*
* @version $Id$
* @since 1.6M1
*/
@SuppressWarnings("checkstyle:ClassFanOutComplexity")
@Component
@Named("hql")
@Singleton
Expand Down Expand Up @@ -93,7 +98,13 @@ public class HqlQueryExecutor implements QueryExecutor, Initializable
@Named("context")
private Provider<ComponentManager> componentManagerProvider;

private volatile Set<String> allowedNamedQueries;
@Inject
private HQLStatementValidator queryValidator;

@Inject
private Logger logger;

private volatile Set<String> safeNamedQueries;

@Override
public void initialize() throws InitializationException
Expand All @@ -103,44 +114,50 @@ public void initialize() throws InitializationException
configuration.addInputStream(Util.getResourceAsStream(MAPPING_PATH));
}

private Set<String> getAllowedNamedQueries()
private Set<String> getSafeNamedQueries()
{
if (this.allowedNamedQueries == null) {
if (this.safeNamedQueries == null) {
synchronized (this) {
if (this.allowedNamedQueries == null) {
this.allowedNamedQueries = new HashSet<>();
if (this.safeNamedQueries == null) {
Set<String> safeQueries = new HashSet<>();

// Gather the list of allowed named queries
Collection<NamedQueryDefinition> namedQueries =
this.hibernate.getConfigurationMetadata().getNamedQueryDefinitions();
for (NamedQueryDefinition definition : namedQueries) {
if (HqlQueryUtils.isSafe(definition.getQuery())) {
this.allowedNamedQueries.add(definition.getName());
try {
if (this.queryValidator.isSafe(definition.getQuery())) {
safeQueries.add(definition.getName());
}
} catch (QueryException e) {
this.logger.warn("Failed to validate named query [{}] with statement [{}]: {}",
definition.getName(), definition.getQuery(), ExceptionUtils.getRootCauseMessage(e));
}
}

this.safeNamedQueries = safeQueries;
}
}
}

return this.allowedNamedQueries;
return this.safeNamedQueries;
}

/**
* @param statementString the statement to evaluate
* @param statement the statement to evaluate
* @return true if the select is allowed for user without PR
* @deprecated
*/
protected static boolean isSafeSelect(String statementString)
@Deprecated(since = "17.0.0RC1, 16.10.2, 15.10.16, 16.4.6")
protected static boolean isSafeSelect(String statement)
{
// An empty statement is safe
if (statementString.isEmpty()) {
return true;
}
HQLStatementValidator validator = Utils.getComponent(HQLStatementValidator.class);

// HqlQueryUtils#isSafe only works with complete statements
String completeStatement = toCompleteShortForm(statementString);

// Parse and validate the statement
return HqlQueryUtils.isSafe(completeStatement);
try {
return validator.isSafe(statement);
} catch (QueryException e) {
return false;
}
}

protected void checkAllowed(final Query query) throws QueryException
Expand All @@ -149,11 +166,11 @@ protected void checkAllowed(final Query query) throws QueryException
if (query instanceof SecureQuery secureQuery && secureQuery.isCurrentAuthorChecked()) {
// Not need to check the details if current author has programming right
if (!this.authorization.hasAccess(Right.PROGRAM)) {
if (query.isNamed() && !getAllowedNamedQueries().contains(query.getStatement())) {
if (query.isNamed() && !getSafeNamedQueries().contains(query.getStatement())) {
throw new QueryException("Named queries requires programming right", query, null);
}

if (!isSafeSelect(query.getStatement())) {
if (!this.queryValidator.isSafe(query.getStatement())) {
throw new QueryException("The query requires programming right", query, null);
}
}
Expand Down Expand Up @@ -200,17 +217,13 @@ public <T> List<T> execute(final Query query) throws QueryException
protected Query filterQuery(Query query)
{
Query filteredQuery = query;

// Named queries are not filtered
if (!filteredQuery.isNamed()) {
// For non-named queries, convert the short form into long form before we apply the filters.
filteredQuery = new WrappingQuery(filteredQuery)
{
@Override
public String getStatement()
{
// handle short queries
return completeShortFormStatement(getWrappedQuery().getStatement());
}
};
// Make sure to work with the complete form of the query
filteredQuery = HqlQueryUtils.toCompleteQuery(filteredQuery);

// Execute filters
filteredQuery = filterQuery(filteredQuery, Query.HQL);
}

Expand Down Expand Up @@ -344,18 +357,7 @@ private boolean hasQueryParametersType(Query query)
*/
protected String completeShortFormStatement(String statement)
{
return toCompleteShortForm(statement);
}

private static String toCompleteShortForm(String statement)
{
String filteredStatement = statement;

if (statement.isEmpty() || HqlQueryUtils.isShortFormStatement(statement)) {
filteredStatement = "select doc.fullName from XWikiDocument doc " + statement.trim();
}

return filteredStatement;
return HqlQueryUtils.toCompleteStatement(statement);
}

private <T> org.hibernate.query.Query<T> createNamedHibernateQuery(Session session, Query query)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.query.hql.internal;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern;

import javax.annotation.Priority;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;

import org.xwiki.component.annotation.Component;
import org.xwiki.component.descriptor.ComponentDescriptor;
import org.xwiki.component.phase.Initializable;
import org.xwiki.component.phase.InitializationException;
import org.xwiki.configuration.ConfigurationSource;

/**
* A validator which can be configured. This validator is executer before the default one using component priorities
*
* @version $Id$
* @since 17.0.0RC1
* @since 16.10.2
* @since 15.10.16
* @since 16.4.6
*/
@Component
@Singleton
@Named("configurable")
@Priority(ComponentDescriptor.DEFAULT_PRIORITY - 100)
public class ConfigurableHQLCompleteStatementValidator implements HQLCompleteStatementValidator, Initializable
{
@Inject
@Named("xwikiproperties")
private ConfigurationSource configuration;

private List<Pattern> unsafe;

private List<Pattern> safe;

@Override
public void initialize() throws InitializationException
{
this.unsafe = getPatterns("query.hql.unsafe");
this.safe = getPatterns("query.hql.safe");
}

private List<Pattern> getPatterns(String key) throws InitializationException
{
List<String> patternStrings = this.configuration.getProperty(key, List.class);

List<Pattern> patterns;
if (patternStrings != null) {
patterns = new ArrayList<>(patternStrings.size());
for (String patternString : patternStrings) {
try {
patterns.add(Pattern.compile(patternString));
} catch (Exception e) {
throw new InitializationException(
String.format("Failed to parse pattern [%s] for configuration [%s]", patternString, key), e);
}
}
} else {
patterns = List.of();
}

return patterns;
}

@Override
public Optional<Boolean> isSafe(String statement)
{
for (Pattern pattern : this.unsafe) {
if (pattern.matcher(statement).matches()) {
return Optional.of(Boolean.FALSE);
}
}

for (Pattern pattern : this.safe) {
if (pattern.matcher(statement).matches()) {
return Optional.of(Boolean.TRUE);
}
}

return Optional.empty();
}
}
Loading

0 comments on commit 0b26038

Please sign in to comment.