CVE-2023-48241: Insufficient Information (NVD-CWE-noinfo)
value: XWiki Platform is a generic wiki platform. Starting in version 6.3-milestone-2 and prior to versions 14.10.15, 15.5.1, and 15.6RC1, the Solr-based search suggestion provider that also duplicates as generic JavaScript API for search results in XWiki exposes the content of all documents of all wikis to anybody who has access to it, by default it is public. This exposes all information stored in the wiki (but not some protected information like password hashes). While there is a right check normally, the right check can be circumvented by explicitly requesting fields from Solr that don't include the data for the right check. This has been fixed in XWiki 15.6RC1, 15.5.1 and 14.10.15 by not listing documents whose rights cannot be checked. No known workarounds are available.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 7.5
Published: 2023-11-20T18:15Z
Repository: https://github.com/xwiki/xwiki-platform
Language: Java
Files changed: xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/main/java/org/xwiki/query/solr/internal/SolrQueryExecutor.java, xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/test/java/org/xwiki/query/solr/SolrQueryExecutorTest.java
Commit date: 2023-07-17 18:06:28 +0200
Commit message: XWIKI-21138: Improve Solr query filtering

Diff: diff --git a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/main/java/org/xwiki/query/solr/internal/SolrQueryExecutor.java b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/main/java/org/xwiki/query/solr/internal/SolrQueryExecutor.java
index 2c9bf514890d6..e263f34dba22e 100644
--- a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/main/java/org/xwiki/query/solr/internal/SolrQueryExecutor.java
+++ b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/main/java/org/xwiki/query/solr/internal/SolrQueryExecutor.java
@@ -231,40 +231,38 @@ private String[] toStringArray(Iterable iterable)
      * @param response the Solr response to filter
      */
     protected void filterResponse(QueryResponse response, List<DocumentReference> usersToCheck)
     {
         SolrDocumentList results = response.getResults();
-        long numFound = results.getNumFound();
+        long numResults = results.size();
 
-        // Since we are modifying the results collection, we need to iterate over its copy.
-        for (SolrDocument result : new ArrayList<SolrDocument>(results)) {
+        results.removeIf(result -> {
+            boolean keep = false;
             try {
                 DocumentReference resultDocumentReference = this.solrDocumentReferenceResolver.resolve(result);
 
-                if (!isAllowed(resultDocumentReference, usersToCheck)) {
+                keep = isAllowed(resultDocumentReference, usersToCheck);
+            } catch (Exception e) {
+                // Don't take any risk of including a result for which we cannot determine the document reference and
+                // thus cannot determine if the given users have access to it or not.
+                this.logger.warn("Removing bad result: {}", result, e);
+            }
 
-                    // Remove the current incompatible result.
-                    results.remove(result);
+            // FIXME: We should update maxScore as well when removing the top scored item. How do we do that?
+            // Sorting based on score might be a not so expensive option.
 
-                    // Decrement the number of results.
-                    numFound--;
+            // FIXME: What about highlighting, facets and all the other data inside the QueryResponse?
 
-                    // FIXME: We should update maxScore as well when removing the top scored item. How do we do that?
-                    // Sorting based on score might be a not so expensive option.
+            return !keep;
+        });
 
-                    // FIXME: What about highlighting, facets and all the other data inside the QueryResponse?
-                }
-            } catch (Exception e) {
-                this.logger.warn("Skipping bad result: {}", result, e);
-            }
-        }
+        long numFilteredResults = numResults - results.size();
+
+        // Update the number of results, excluding the filtered ones.
+        // Lower bound guard for the total number of results.
+        long numFound = Math.max(0, response.getResults().getNumFound() - numFilteredResults);
 
-        // Update the new number of results, excluding the filtered ones.
-        if (numFound < 0) {
-            // Lower bound guard for the total number of results.
-            numFound = 0;
-        }
         results.setNumFound(numFound);
     }
 
     protected boolean isAllowed(DocumentReference resultDocumentReference, List<DocumentReference> usersToCheck)
     {
diff --git a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/test/java/org/xwiki/query/solr/SolrQueryExecutorTest.java b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/test/java/org/xwiki/query/solr/SolrQueryExecutorTest.java
index d556f40d992cf..695f3d672f46e 100644
--- a/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/test/java/org/xwiki/query/solr/SolrQueryExecutorTest.java
+++ b/xwiki-platform-core/xwiki-platform-search/xwiki-platform-search-solr/xwiki-platform-search-solr-query/src/test/java/org/xwiki/query/solr/SolrQueryExecutorTest.java
@@ -19,10 +19,11 @@
  */
 package org.xwiki.query.solr;
 
 import java.lang.reflect.ParameterizedType;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Locale;
 
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.common.SolrDocument;
@@ -70,23 +71,23 @@
 @ReferenceComponentList
 public class SolrQueryExecutorTest
 {
     private static final String ITERABLE_PARAM_NAME = "multiParam";
 
-    private static final String[] ITERABLE_PARAM_EXPECTED = {"value1", "value2"};
+    private static final String[] ITERABLE_PARAM_EXPECTED = { "value1", "value2" };
 
     private static final Iterable<String> ITERABLE_PARAM_VALUE = Arrays.asList(ITERABLE_PARAM_EXPECTED);
 
     private static final String INT_ARR_PARAM_NAME = "intArrayParam";
 
-    private static final String[] INT_ARR_PARAM_EXPECTED = {"-42", "4711"};
+    private static final String[] INT_ARR_PARAM_EXPECTED = { "-42", "4711" };
 
-    private static final int[] INT_ARR_PARAM_VALUE = {-42, 4711};
+    private static final int[] INT_ARR_PARAM_VALUE = { -42, 4711 };
 
     private static final String STR_ARR_PARAM_NAME = "stringArrayParam";
 
-    private static final String[] STR_ARR_PARAM_EXPECTED = {"valueA", "valueB"};
+    private static final String[] STR_ARR_PARAM_EXPECTED = { "valueA", "valueB" };
 
     private static final String[] STR_ARR_PARAM_VALUE = STR_ARR_PARAM_EXPECTED;
 
     private static final String SINGLE_PARAM_NAME = "singleParam";
 
@@ -226,6 +227,64 @@ public void filterResponse() throws Exception
         when(response.getResults()).thenReturn((SolrDocumentList) sourceResults.clone());
 
         results = ((QueryResponse) this.componentManager.getComponentUnderTest().execute(query).get(0)).getResults();
         assertEquals(Arrays.asList(alice, bob), results);
     }
-}
+
+    @Test
+    public void filterResponseWithException() throws Exception
+    {
+        ParameterizedType resolverType =
+            new DefaultParameterizedType(null, DocumentReferenceResolver.class, SolrDocument.class);
+        DocumentReferenceResolver<SolrDocument> resolver = this.componentManager.getInstance(resolverType);
+
+        AuthorizationManager authorizationManager = this.componentManager.getInstance(AuthorizationManager.class);
+
+        DocumentReference currentUserReference = new DocumentReference("xwiki", "XWiki", "currentuser");
+        this.oldCore.getXWikiContext().setUserReference(currentUserReference);
+
+        DocumentReference aliceReference = new DocumentReference("wiki", "Users", "Alice");
+        SolrDocument alice = new SolrDocument();
+        when(resolver.resolve(alice)).thenReturn(aliceReference);
+
+        DocumentReference bobReference = new DocumentReference("wiki", "Users", "Bob");
+        when(authorizationManager.hasAccess(Right.VIEW, currentUserReference, bobReference)).thenReturn(true);
+        SolrDocument bob = new SolrDocument();
+        when(resolver.resolve(bob)).thenReturn(bobReference);
+
+        SolrDocumentList sourceResults = new SolrDocumentList();
+        sourceResults.addAll(Arrays.asList(alice, bob));
+        sourceResults.setNumFound(2);
+
+        QueryResponse response = mock(QueryResponse.class);
+        when(this.solr.query(any(SolrParams.class))).thenReturn(response);
+
+        DefaultQuery query = new DefaultQuery("", null);
+
+        // No right check, verify that the setup works.
+        when(response.getResults()).thenReturn((SolrDocumentList) sourceResults.clone());
+        SolrDocumentList results =
+            ((QueryResponse) this.componentManager.getComponentUnderTest().execute(query).get(0)).getResults();
+        assertEquals(Arrays.asList(alice, bob), results);
+        assertEquals(2, results.getNumFound());
+
+        // Check current user right
+        query.checkCurrentUser(true);
+
+        // Throw an exception when resolving Alice
+        when(resolver.resolve(alice)).thenThrow(new RuntimeException("Alice"));
+        when(response.getResults()).thenReturn((SolrDocumentList) sourceResults.clone());
+
+        results = ((QueryResponse) this.componentManager.getComponentUnderTest().execute(query).get(0)).getResults();
+        assertEquals(Collections.singletonList(bob), results);
+        assertEquals(1, results.getNumFound());
+
+        // Throw also an exception when resolving Bob
+        when(resolver.resolve(bob)).thenThrow(new RuntimeException("Bob"));
+        when(response.getResults()).thenReturn((SolrDocumentList) sourceResults.clone());
+
+        // Assert that the results are empty when both throw an exception
+        results = ((QueryResponse) this.componentManager.getComponentUnderTest().execute(query).get(0)).getResults();
+        assertEquals(Collections.emptyList(), results);
+        assertEquals(0, results.getNumFound());
+    }
+}
\ No newline at end of file

Vulnerable code:         long numFound = results.getNumFound();
        // Since we are modifying the results collection, we need to iterate over its copy.
        for (SolrDocument result : new ArrayList<SolrDocument>(results)) {
                if (!isAllowed(resultDocumentReference, usersToCheck)) {
                    // Remove the current incompatible result.
                    results.remove(result);
                    // Decrement the number of results.
                    numFound--;
                    // FIXME: We should update maxScore as well when removing the top scored item. How do we do that?
                    // Sorting based on score might be a not so expensive option.
                    // FIXME: What about highlighting, facets and all the other data inside the QueryResponse?
                }
            } catch (Exception e) {
                this.logger.warn("Skipping bad result: {}", result, e);
            }
        }
        // Update the new number of results, excluding the filtered ones.
        if (numFound < 0) {
            // Lower bound guard for the total number of results.
            numFound = 0;
        }
    private static final String[] ITERABLE_PARAM_EXPECTED = {"value1", "value2"};
    private static final String[] INT_ARR_PARAM_EXPECTED = {"-42", "4711"};
    private static final int[] INT_ARR_PARAM_VALUE = {-42, 4711};
    private static final String[] STR_ARR_PARAM_EXPECTED = {"valueA", "valueB"};
}
Fixed code:         long numResults = results.size();
        results.removeIf(result -> {
            boolean keep = false;
                keep = isAllowed(resultDocumentReference, usersToCheck);
            } catch (Exception e) {
                // Don't take any risk of including a result for which we cannot determine the document reference and
                // thus cannot determine if the given users have access to it or not.
                this.logger.warn("Removing bad result: {}", result, e);
            }
            // FIXME: We should update maxScore as well when removing the top scored item. How do we do that?
            // Sorting based on score might be a not so expensive option.
            // FIXME: What about highlighting, facets and all the other data inside the QueryResponse?
            return !keep;
        });
        long numFilteredResults = numResults - results.size();

        // Update the number of results, excluding the filtered ones.
        // Lower bound guard for the total number of results.
        long numFound = Math.max(0, response.getResults().getNumFound() - numFilteredResults);
import java.util.Collections;
    private static final String[] ITERABLE_PARAM_EXPECTED = { "value1", "value2" };
    private static final String[] INT_ARR_PARAM_EXPECTED = { "-42", "4711" };
    private static final int[] INT_ARR_PARAM_VALUE = { -42, 4711 };
    private static final String[] STR_ARR_PARAM_EXPECTED = { "valueA", "valueB" };

    @Test
    public void filterResponseWithException() throws Exception
    {
        ParameterizedType resolverType =
            new DefaultParameterizedType(null, DocumentReferenceResolver.class, SolrDocument.class);
        DocumentReferenceResolver<SolrDocument> resolver = this.componentManager.getInstance(resolverType);

        AuthorizationManager authorizationManager = this.componentManager.getInstance(AuthorizationManager.class);

        DocumentReference currentUserReference = new DocumentReference("xwiki", "XWiki", "currentuser");
        this.oldCore.getXWikiContext().setUserReference(currentUserReference);

        DocumentReference aliceReference = new DocumentReference("wiki", "Users", "Alice");
        SolrDocument alice = new SolrDocument();
        when(resolver.resolve(alice)).thenReturn(aliceReference);

        DocumentReference bobReference = new DocumentReference("wiki", "Users", "Bob");
        when(authorizationManager.hasAccess(Right.VIEW, currentUserReference, bobReference)).thenReturn(true);
        SolrDocument bob = new SolrDocument();
        when(resolver.resolve(bob)).thenReturn(bobReference);

        SolrDocumentList sourceResults = new SolrDocumentList();
        sourceResults.addAll(Arrays.asList(alice, bob));
        sourceResults.setNumFound(2);

        QueryResponse response = mock(QueryResponse.class);
        when(this.solr.query(any(SolrParams.class))).thenReturn(response);

        DefaultQuery query = new DefaultQuery("", null);

        // No right check, verify that the setup works.
        when(response.getResults()).thenReturn((SolrDocumentList) sourceResults.clone());
        SolrDocumentList results =
            ((QueryResponse) this.componentManager.getComponentUnderTest().execute(query).get(0)).getResults();
        assertEquals(Arrays.asList(alice, bob), results);
        assertEquals(2, results.getNumFound());

        // Check current user right
        query.checkCurrentUser(true);

        // Throw an exception when resolving Alice
        when(resolver.resolve(alice)).thenThrow(new RuntimeException("Alice"));
        when(response.getResults()).thenReturn((SolrDocumentList) sourceResults.clone());

        results = ((QueryResponse) this.componentManager.getComponentUnderTest().execute(query).get(0)).getResults();
        assertEquals(Collections.singletonList(bob), results);
        assertEquals(1, results.getNumFound());

        // Throw also an exception when resolving Bob
        when(resolver.resolve(bob)).thenThrow(new RuntimeException("Bob"));
        when(response.getResults()).thenReturn((SolrDocumentList) sourceResults.clone());

        // Assert that the results are empty when both throw an exception
        results = ((QueryResponse) this.componentManager.getComponentUnderTest().execute(query).get(0)).getResults();
        assertEquals(Collections.emptyList(), results);
        assertEquals(0, results.getNumFound());
    }
}