Skip to content

Commit 7313dc9

Browse files
tmortagnetmortagne
committed
XWIKI-22728: Improve search query validation
1 parent ca930fb commit 7313dc9

File tree

3 files changed

+137
-49
lines changed

3 files changed

+137
-49
lines changed

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/api/XWiki.java

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.xwiki.model.reference.PageReference;
4545
import org.xwiki.model.reference.SpaceReference;
4646
import org.xwiki.model.reference.WikiReference;
47+
import org.xwiki.query.hql.internal.HQLStatementValidator;
4748
import org.xwiki.rendering.renderer.PrintRendererFactory;
4849
import org.xwiki.rendering.syntax.Syntax;
4950
import org.xwiki.security.authorization.AuthorizationException;
@@ -58,6 +59,7 @@
5859
import com.xpn.xwiki.doc.XWikiDocument;
5960
import com.xpn.xwiki.internal.XWikiInitializerJob;
6061
import com.xpn.xwiki.internal.XWikiInitializerJobStatus;
62+
import com.xpn.xwiki.internal.store.hibernate.query.HqlQueryUtils;
6163
import com.xpn.xwiki.objects.meta.MetaClass;
6264
import com.xpn.xwiki.user.api.XWikiUser;
6365
import com.xpn.xwiki.util.Programming;
@@ -105,6 +107,8 @@ public class XWiki extends Api
105107

106108
private ContextualAuthorizationManager contextualAuthorizationManager;
107109

110+
private HQLStatementValidator hqlValidator;
111+
108112
/**
109113
* XWiki API Constructor
110114
*
@@ -167,6 +171,15 @@ private DocumentRevisionProvider getDocumentRevisionProvider()
167171
return this.documentRevisionProvider;
168172
}
169173

174+
private HQLStatementValidator getHQLStatementValidator()
175+
{
176+
if (this.hqlValidator == null) {
177+
this.hqlValidator = Utils.getComponent(HQLStatementValidator.class);
178+
}
179+
180+
return this.hqlValidator;
181+
}
182+
170183
/**
171184
* Privileged API allowing to access the underlying main XWiki Object
172185
*
@@ -710,6 +723,23 @@ public MetaClass getMetaclass()
710723
return this.xwiki.getMetaclass();
711724
}
712725

726+
private void checkSearchQueryAllowed(String whereSQL) throws XWikiException
727+
{
728+
if (!hasProgrammingRights()) {
729+
try {
730+
if (!getHQLStatementValidator()
731+
.isSafe(HqlQueryUtils.createLegacySQLQuery("select distinct doc.fullName", whereSQL))) {
732+
throw new XWikiException(XWikiException.MODULE_XWIKI_STORE,
733+
XWikiException.ERROR_XWIKI_ACCESS_DENIED,
734+
"The query [" + whereSQL + "] requires programming right");
735+
}
736+
} catch (Exception e) {
737+
throw new XWikiException(XWikiException.MODULE_XWIKI_STORE, XWikiException.ERROR_XWIKI_ACCESS_DENIED,
738+
"Failed to validate the query [" + whereSQL + "], requiring programming right", e);
739+
}
740+
}
741+
}
742+
713743
/**
714744
* API allowing to search for document names matching a query. Examples:
715745
* <ul>
@@ -742,6 +772,8 @@ public MetaClass getMetaclass()
742772
@Deprecated
743773
public List<String> searchDocuments(String wheresql) throws XWikiException
744774
{
775+
checkSearchQueryAllowed(wheresql);
776+
745777
return this.xwiki.getStore().searchDocumentsNames(wheresql, getXWikiContext());
746778
}
747779

@@ -760,6 +792,8 @@ public List<String> searchDocuments(String wheresql) throws XWikiException
760792
@Deprecated
761793
public List<String> searchDocuments(String wheresql, int nb, int start) throws XWikiException
762794
{
795+
checkSearchQueryAllowed(wheresql);
796+
763797
return this.xwiki.getStore().searchDocumentsNames(wheresql, nb, start, getXWikiContext());
764798
}
765799

@@ -796,6 +830,8 @@ public List<String> searchDocuments(String wheresql, int nb, int start, String s
796830
*/
797831
public List<Document> searchDocuments(String wheresql, boolean distinctbylocale) throws XWikiException
798832
{
833+
checkSearchQueryAllowed(wheresql);
834+
799835
return convert(this.xwiki.getStore().searchDocuments(wheresql, distinctbylocale, getXWikiContext()));
800836
}
801837

@@ -812,6 +848,8 @@ public List<Document> searchDocuments(String wheresql, boolean distinctbylocale)
812848
public List<Document> searchDocuments(String wheresql, boolean distinctbylocale, int nb, int start)
813849
throws XWikiException
814850
{
851+
checkSearchQueryAllowed(wheresql);
852+
815853
return convert(this.xwiki.getStore().searchDocuments(wheresql, distinctbylocale, nb, start, getXWikiContext()));
816854
}
817855

@@ -845,6 +883,8 @@ public List<Document> searchDocuments(String wheresql, boolean distinctbylocale,
845883
public List<String> searchDocuments(String parameterizedWhereClause, int maxResults, int startOffset,
846884
List<?> parameterValues) throws XWikiException
847885
{
886+
checkSearchQueryAllowed(parameterizedWhereClause);
887+
848888
return this.xwiki.getStore().searchDocumentsNames(parameterizedWhereClause, maxResults, startOffset,
849889
parameterValues, getXWikiContext());
850890
}
@@ -858,6 +898,8 @@ public List<String> searchDocuments(String parameterizedWhereClause, int maxResu
858898
@Deprecated
859899
public List<String> searchDocuments(String parameterizedWhereClause, List<?> parameterValues) throws XWikiException
860900
{
901+
checkSearchQueryAllowed(parameterizedWhereClause);
902+
861903
return this.xwiki.getStore().searchDocumentsNames(parameterizedWhereClause, parameterValues, getXWikiContext());
862904
}
863905

@@ -885,6 +927,8 @@ public List<String> searchDocumentsNames(String wikiName, String parameterizedWh
885927
try {
886928
this.context.setWikiId(wikiName);
887929

930+
checkSearchQueryAllowed(parameterizedWhereClause);
931+
888932
return searchDocuments(parameterizedWhereClause, maxResults, startOffset, parameterValues);
889933
} finally {
890934
this.context.setWikiId(database);
@@ -895,18 +939,20 @@ public List<String> searchDocumentsNames(String wikiName, String parameterizedWh
895939
* Search spaces by passing HQL where clause values as parameters. See
896940
* {@link #searchDocuments(String, int, int, List)} for more about parameterized hql clauses.
897941
*
898-
* @param parametrizedSqlClause the HQL where clause. For example
942+
* @param parameterizedSqlClause the HQL where clause. For example
899943
* {@code where doc.fullName <> ?1 and (doc.parent = ?2 or (doc.parent = ?3 and doc.space = ?4))}
900944
* @param nb the number of rows to return. If 0 then all rows are returned
901945
* @param start the number of rows to skip. If 0 don't skip any row
902946
* @param parameterValues the where clause values that replace the question marks (?)
903947
* @return a list of spaces names.
904948
* @throws XWikiException in case of error while performing the query
905949
*/
906-
public List<String> searchSpacesNames(String parametrizedSqlClause, int nb, int start, List<?> parameterValues)
950+
public List<String> searchSpacesNames(String parameterizedSqlClause, int nb, int start, List<?> parameterValues)
907951
throws XWikiException
908952
{
909-
return this.xwiki.getStore().search("select distinct doc.space from XWikiDocument doc " + parametrizedSqlClause,
953+
checkSearchQueryAllowed(parameterizedSqlClause);
954+
955+
return this.xwiki.getStore().search("select distinct doc.space from XWikiDocument doc " + parameterizedSqlClause,
910956
nb, start, parameterValues, this.context);
911957
}
912958

@@ -915,7 +961,7 @@ public List<String> searchSpacesNames(String parametrizedSqlClause, int nb, int
915961
* {@link #searchDocuments(String, int, int, List)} for more about parameterized hql clauses. You can specify
916962
* properties of attach (the attachment) or doc (the document it is attached to)
917963
*
918-
* @param parametrizedSqlClause The HQL where clause. For example
964+
* @param parameterizedSqlClause The HQL where clause. For example
919965
* {@code where doc.fullName <> ?1 and (doc.parent = ?2 or (doc.parent = ?3 and doc.space = ?4))}
920966
* @param nb The number of rows to return. If 0 then all rows are returned
921967
* @param start The number of rows to skip at the beginning.
@@ -924,27 +970,31 @@ public List<String> searchSpacesNames(String parametrizedSqlClause, int nb, int
924970
* @throws XWikiException in case of error while performing the query
925971
* @since 5.0M2
926972
*/
927-
public List<Attachment> searchAttachments(String parametrizedSqlClause, int nb, int start, List<?> parameterValues)
973+
public List<Attachment> searchAttachments(String parameterizedSqlClause, int nb, int start, List<?> parameterValues)
928974
throws XWikiException
929975
{
976+
checkSearchQueryAllowed(parameterizedSqlClause);
977+
930978
return convertAttachments(
931-
this.xwiki.searchAttachments(parametrizedSqlClause, true, nb, start, parameterValues, this.context));
979+
this.xwiki.searchAttachments(parameterizedSqlClause, true, nb, start, parameterValues, this.context));
932980
}
933981

934982
/**
935983
* Count attachments returned by a given parameterized query
936984
*
937-
* @param parametrizedSqlClause Everything which would follow the "WHERE" in HQL see:
985+
* @param parameterizedSqlClause Everything which would follow the "WHERE" in HQL see:
938986
* {@link #searchDocuments(String, int, int, List)}
939987
* @param parameterValues A {@link java.util.List} of the where clause values that replace the question marks (?)
940988
* @return int number of attachments found.
941989
* @throws XWikiException
942990
* @see #searchAttachments(String, int, int, List)
943991
* @since 5.0M2
944992
*/
945-
public int countAttachments(String parametrizedSqlClause, List<?> parameterValues) throws XWikiException
993+
public int countAttachments(String parameterizedSqlClause, List<?> parameterValues) throws XWikiException
946994
{
947-
return this.xwiki.countAttachments(parametrizedSqlClause, parameterValues, this.context);
995+
checkSearchQueryAllowed(parameterizedSqlClause);
996+
997+
return this.xwiki.countAttachments(parameterizedSqlClause, parameterValues, this.context);
948998
}
949999

9501000
/**

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/store/hibernate/query/HqlQueryUtils.java

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
*/
2020
package com.xpn.xwiki.internal.store.hibernate.query;
2121

22+
import java.util.StringTokenizer;
2223
import java.util.regex.Matcher;
2324
import java.util.regex.Pattern;
2425

@@ -34,6 +35,16 @@
3435
*/
3536
public final class HqlQueryUtils
3637
{
38+
private static final String FROM = "from";
39+
40+
private static final String WHERE = "where ";
41+
42+
private static final String ORDER = "order";
43+
44+
private static final String ORDER_BY = "order by";
45+
46+
private static final String COMMA = ",";
47+
3748
private static final Pattern LEGACY_ORDINAL_PARAMS_PATTERN = Pattern.compile("([=\\s,\\(<>])\\?([=\\s,\\)<>]|$)");
3849

3950
private HqlQueryUtils()
@@ -47,7 +58,7 @@ private HqlQueryUtils()
4758
*/
4859
public static boolean isShortFormStatement(String statement)
4960
{
50-
return StringUtils.startsWithAny(statement.trim().toLowerCase(), ",", "where ", "order by");
61+
return StringUtils.startsWithAny(statement.trim().toLowerCase(), COMMA, WHERE, ORDER_BY);
5162
}
5263

5364
/**
@@ -129,4 +140,67 @@ public String getStatement()
129140

130141
return completeQuery;
131142
}
143+
144+
/**
145+
* @param queryPrefix the start of the SQL query (for example "select distinct doc.space, doc.name")
146+
* @param whereSQL the where clause to append
147+
* @return the full formed SQL query, to which the order by columns have been added as returned columns (this is
148+
* required for example for HSQLDB).
149+
* @since 17.0.3RC1
150+
* @since 16.10.6
151+
*/
152+
public static String createLegacySQLQuery(String queryPrefix, String whereSQL)
153+
{
154+
StringBuilder sql = new StringBuilder(queryPrefix);
155+
156+
String normalizedWhereSQL;
157+
if (StringUtils.isBlank(whereSQL)) {
158+
normalizedWhereSQL = "";
159+
} else {
160+
normalizedWhereSQL = whereSQL.trim();
161+
}
162+
163+
sql.append(getColumnsForSelectStatement(normalizedWhereSQL));
164+
sql.append(" from XWikiDocument as doc");
165+
166+
if (!normalizedWhereSQL.equals("")) {
167+
if ((!normalizedWhereSQL.startsWith(WHERE)) && (normalizedWhereSQL.charAt(0) != ',')) {
168+
sql.append(" where ");
169+
} else {
170+
sql.append(" ");
171+
}
172+
sql.append(normalizedWhereSQL);
173+
}
174+
175+
return sql.toString();
176+
}
177+
178+
/**
179+
* @param whereSQL the SQL where clause
180+
* @return the list of columns to return in the select clause as a string starting with ", " if there are columns or
181+
* an empty string otherwise. The returned columns are extracted from the where clause. One reason for doing
182+
* so is because HSQLDB only support SELECT DISTINCT SQL statements where the columns operated on are
183+
* returned from the query.
184+
* @since 17.0.3RC1
185+
* @since 16.10.6
186+
*/
187+
public static String getColumnsForSelectStatement(String whereSQL)
188+
{
189+
StringBuilder columns = new StringBuilder();
190+
191+
int orderByPos = whereSQL.toLowerCase().indexOf(ORDER_BY);
192+
if (orderByPos >= 0) {
193+
String orderByStatement = whereSQL.substring(orderByPos + ORDER_BY.length() + 1);
194+
StringTokenizer tokenizer = new StringTokenizer(orderByStatement, COMMA);
195+
while (tokenizer.hasMoreTokens()) {
196+
String column = tokenizer.nextToken().trim();
197+
// Remove "desc" or "asc" from the column found
198+
column = StringUtils.removeEndIgnoreCase(column, " desc");
199+
column = StringUtils.removeEndIgnoreCase(column, " asc");
200+
columns.append(", ").append(column.trim());
201+
}
202+
}
203+
204+
return columns.toString();
205+
}
132206
}

‎xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateStore.java

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import java.util.Objects;
3939
import java.util.Optional;
4040
import java.util.Set;
41-
import java.util.StringTokenizer;
4241
import java.util.concurrent.atomic.AtomicReference;
4342
import java.util.concurrent.locks.Lock;
4443
import java.util.concurrent.locks.ReentrantLock;
@@ -101,6 +100,7 @@
101100
import com.xpn.xwiki.doc.XWikiLock;
102101
import com.xpn.xwiki.doc.XWikiSpace;
103102
import com.xpn.xwiki.internal.store.hibernate.legacy.LegacySessionImplementor;
103+
import com.xpn.xwiki.internal.store.hibernate.query.HqlQueryUtils;
104104
import com.xpn.xwiki.monitor.api.MonitorPlugin;
105105
import com.xpn.xwiki.objects.BaseCollection;
106106
import com.xpn.xwiki.objects.BaseElement;
@@ -2916,28 +2916,7 @@ public List<XWikiDocument> searchDocuments(String wheresql, boolean distinctbyla
29162916
*/
29172917
protected String createSQLQuery(String queryPrefix, String whereSQL)
29182918
{
2919-
StringBuilder sql = new StringBuilder(queryPrefix);
2920-
2921-
String normalizedWhereSQL;
2922-
if (StringUtils.isBlank(whereSQL)) {
2923-
normalizedWhereSQL = "";
2924-
} else {
2925-
normalizedWhereSQL = whereSQL.trim();
2926-
}
2927-
2928-
sql.append(getColumnsForSelectStatement(normalizedWhereSQL));
2929-
sql.append(" from XWikiDocument as doc");
2930-
2931-
if (!normalizedWhereSQL.equals("")) {
2932-
if ((!normalizedWhereSQL.startsWith("where")) && (!normalizedWhereSQL.startsWith(","))) {
2933-
sql.append(" where ");
2934-
} else {
2935-
sql.append(" ");
2936-
}
2937-
sql.append(normalizedWhereSQL);
2938-
}
2939-
2940-
return sql.toString();
2919+
return HqlQueryUtils.createLegacySQLQuery(queryPrefix, whereSQL);
29412920
}
29422921

29432922
/**
@@ -2949,22 +2928,7 @@ protected String createSQLQuery(String queryPrefix, String whereSQL)
29492928
*/
29502929
protected String getColumnsForSelectStatement(String whereSQL)
29512930
{
2952-
StringBuilder columns = new StringBuilder();
2953-
2954-
int orderByPos = whereSQL.toLowerCase().indexOf("order by");
2955-
if (orderByPos >= 0) {
2956-
String orderByStatement = whereSQL.substring(orderByPos + "order by".length() + 1);
2957-
StringTokenizer tokenizer = new StringTokenizer(orderByStatement, ",");
2958-
while (tokenizer.hasMoreTokens()) {
2959-
String column = tokenizer.nextToken().trim();
2960-
// Remove "desc" or "asc" from the column found
2961-
column = StringUtils.removeEndIgnoreCase(column, " desc");
2962-
column = StringUtils.removeEndIgnoreCase(column, " asc");
2963-
columns.append(", ").append(column.trim());
2964-
}
2965-
}
2966-
2967-
return columns.toString();
2931+
return HqlQueryUtils.getColumnsForSelectStatement(whereSQL);
29682932
}
29692933

29702934
@Override

0 commit comments

Comments
 (0)