CVE-2022-39387: Improper Authentication (CWE-287)
value: XWiki OIDC has various tools to manipulate OpenID Connect protocol in XWiki. Prior to version 1.29.1, even if a wiki has an OpenID provider configured through its xwiki.properties, it is possible to provide a third party provider its details through request parameters. One can then bypass the XWiki authentication altogether by specifying its own provider through the oidc.endpoint.* request parameters (or by using an XWiki-based OpenID provider with oidc.xwikiprovider. With the same approach, one could also provide a specific group mapping through oidc.groups.mapping that would make his user automatically part of the XWikiAdminGroup. This issue has been patched, please upgrade to 1.29.1. There is no workaround, an upgrade of the authenticator is required.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 7.5
Published: 2022-11-04T19:15Z
Repository: https://github.com/xwiki-contrib/oidc
Language: Java
Files changed: oidc-authenticator/src/main/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfiguration.java, oidc-authenticator/src/test/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfigurationTest.java
Commit date: 2022-01-27 15:16:30 +0100
Commit message: OIDC-118: Fix bad handling of request parameters

Diff: diff --git a/oidc-authenticator/src/main/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfiguration.java b/oidc-authenticator/src/main/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfiguration.java
index 8fba1dc..417ee20 100644
--- a/oidc-authenticator/src/main/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfiguration.java
+++ b/oidc-authenticator/src/main/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfiguration.java
@@ -35,15 +35,15 @@ import java.util.Set;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import javax.servlet.http.HttpSession;
 
 import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.collections4.SetUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.joda.time.LocalDateTime;
 import org.slf4j.Logger;
 import org.xwiki.component.annotation.Component;
-import org.xwiki.configuration.ConfigurationSource;
 import org.xwiki.container.Container;
 import org.xwiki.container.Request;
 import org.xwiki.container.Session;
 import org.xwiki.container.servlet.ServletSession;
 import org.xwiki.contrib.oidc.OIDCIdToken;
@@ -232,10 +232,12 @@ public class OIDCClientConfiguration extends OIDCConfiguration
 
     public static final String PROP_SESSION_USERINFO_EXPORATIONDATE = "oidc.session.userinfoexpirationdate";
 
     private static final String XWIKI_GROUP_PREFIX = "XWiki.";
 
+    private static final Set<String> SAFE_PROPERTIES = SetUtils.hashSet(PROP_SKIPPED);
+
     @Inject
     private InstanceIdManager instance;
 
     @Inject
     private OIDCManager manager;
@@ -247,14 +249,10 @@ public class OIDCClientConfiguration extends OIDCConfiguration
     private ConverterManager converter;
 
     @Inject
     private Logger logger;
 
-    @Inject
-    // TODO: store configuration in custom objects
-    private ConfigurationSource configuration;
-
     private HttpSession getHttpSession()
     {
         Session session = this.container.getSession();
         if (session instanceof ServletSession) {
             HttpSession httpSession = ((ServletSession) session).getHttpSession();
@@ -333,14 +331,16 @@ public class OIDCClientConfiguration extends OIDCConfiguration
     }
 
     @Override
     protected <T> T getProperty(String key, Class<T> valueClass)
     {
-        // Get property from request
-        String requestValue = getRequestParameter(key);
-        if (requestValue != null) {
-            return this.converter.convert(valueClass, requestValue);
+        if (SAFE_PROPERTIES.contains(key)) {
+            // Get property from request
+            String requestValue = getRequestParameter(key);
+            if (requestValue != null) {
+                return this.converter.convert(valueClass, requestValue);
+            }
         }
 
         // Get property from session
         T sessionValue = getSessionAttribute(key);
         if (sessionValue != null) {
@@ -352,14 +352,16 @@ public class OIDCClientConfiguration extends OIDCConfiguration
     }
 
     @Override
     protected <T> T getProperty(String key, T def)
     {
-        // Get property from request
-        String requestValue = getRequestParameter(key);
-        if (requestValue != null) {
-            return this.converter.convert(def.getClass(), requestValue);
+        if (SAFE_PROPERTIES.contains(key)) {
+            // Get property from request
+            String requestValue = getRequestParameter(key);
+            if (requestValue != null) {
+                return this.converter.convert(def.getClass(), requestValue);
+            }
         }
 
         // Get property from session
         T sessionValue = getSessionAttribute(key);
         if (sessionValue != null) {
@@ -424,13 +426,23 @@ public class OIDCClientConfiguration extends OIDCConfiguration
             }
         } else {
             uri = new URI(uriString);
         }
 
-        // If we still don't have any endpoint URI, return null
+        // If we still don't have any endpoint URI, try the request
         if (uri == null) {
-            return null;
+            uriString = getRequestParameter(PROPPREFIX_ENDPOINT + hint);
+            if (uriString == null) {
+                String provider = getRequestParameter(PROP_XWIKIPROVIDER);
+                if (provider == null) {
+                    return null;
+                }
+
+                uri = this.manager.createEndPointURI(provider, hint);
+            } else {
+                uri = new URI(uriString);
+            }
         }
 
         // Find custom headers
         Map<String, List<String>> headers = new LinkedHashMap<>();
 
diff --git a/oidc-authenticator/src/test/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfigurationTest.java b/oidc-authenticator/src/test/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfigurationTest.java
index e4bccbd..dea5711 100644
--- a/oidc-authenticator/src/test/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfigurationTest.java
+++ b/oidc-authenticator/src/test/java/org/xwiki/contrib/oidc/auth/internal/OIDCClientConfigurationTest.java
@@ -17,24 +17,34 @@
  * 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.contrib.oidc.auth.internal;
 
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URL;
 import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
 import org.junit.jupiter.api.Test;
 import org.xwiki.configuration.ConfigurationSource;
+import org.xwiki.container.Container;
+import org.xwiki.container.servlet.ServletRequest;
+import org.xwiki.contrib.oidc.provider.internal.OIDCManager;
+import org.xwiki.contrib.oidc.provider.internal.endpoint.TokenOIDCEndpoint;
+import org.xwiki.properties.ConverterManager;
 import org.xwiki.test.junit5.mockito.ComponentTest;
 import org.xwiki.test.junit5.mockito.InjectMockComponents;
 import org.xwiki.test.junit5.mockito.MockComponent;
 
+import com.xpn.xwiki.web.XWikiServletRequestStub;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 
 /**
@@ -49,10 +59,19 @@ class OIDCClientConfigurationTest
     private OIDCClientConfiguration configuration;
 
     @MockComponent
     private ConfigurationSource sourceConfiguration;
 
+    @MockComponent
+    private Container container;
+
+    @MockComponent
+    private OIDCManager manager;
+
+    @MockComponent
+    private ConverterManager converterManager;
+
     @Test
     void getUserInfoOIDCEndpoint() throws URISyntaxException
     {
         assertNull(this.configuration.getUserInfoOIDCEndpoint());
 
@@ -76,8 +95,41 @@ class OIDCClientConfigurationTest
 
         endpoint = this.configuration.getUserInfoOIDCEndpoint();
 
         assertEquals(uri, endpoint.getURI());
         assertEquals(headers, endpoint.getHeaders());
+    }
+
+    @Test
+    void getPropertyOrder() throws MalformedURLException, URISyntaxException
+    {
+        String provider = "http://urlprovider";
+        URI urlauthorization = new URI("http://urlauthorization");
+
+        XWikiServletRequestStub requestStub = new XWikiServletRequestStub(new URL("http://url"), null);
+
+        when(this.container.getRequest()).thenReturn(new ServletRequest(requestStub));
+        when(this.sourceConfiguration.getProperty(OIDCClientConfiguration.PROP_SKIPPED, false)).thenReturn(false);
+
+        assertFalse(this.configuration.isSkipped());
+        assertNull(this.configuration.getXWikiProvider());
+        assertNull(this.configuration.getAuthorizationOIDCEndpoint());
+        assertNull(this.configuration.getAuthorizationOIDCEndpoint());
+        assertNull(this.configuration.getTokenOIDCEndpoint());
+
+        requestStub.put(OIDCClientConfiguration.PROP_SKIPPED, "true");
+        when(this.converterManager.convert(Boolean.class, "true")).thenReturn(true);
+
+        assertTrue(this.configuration.isSkipped());
+
+        requestStub.put(OIDCClientConfiguration.PROP_GROUPS_ALLOWED, "true");
+
+        assertNull(this.configuration.getAllowedGroups());
+
+        requestStub.put(OIDCClientConfiguration.PROP_XWIKIPROVIDER, provider.toString());
+        requestStub.put(OIDCClientConfiguration.PROP_ENDPOINT_AUTHORIZATION, urlauthorization.toString());
+        when(this.manager.createEndPointURI(provider, TokenOIDCEndpoint.HINT)).thenReturn(new URI(provider));
 
+        assertEquals(urlauthorization, this.configuration.getAuthorizationOIDCEndpoint().getURI());
+        assertEquals(provider, this.configuration.getTokenOIDCEndpoint().getURI().toString());
     }
 }

Vulnerable code: import org.xwiki.configuration.ConfigurationSource;
    @Inject
    // TODO: store configuration in custom objects
    private ConfigurationSource configuration;

        // Get property from request
        String requestValue = getRequestParameter(key);
        if (requestValue != null) {
            return this.converter.convert(valueClass, requestValue);
        // Get property from request
        String requestValue = getRequestParameter(key);
        if (requestValue != null) {
            return this.converter.convert(def.getClass(), requestValue);
        // If we still don't have any endpoint URI, return null
            return null;
Fixed code: import org.apache.commons.collections4.SetUtils;
    private static final Set<String> SAFE_PROPERTIES = SetUtils.hashSet(PROP_SKIPPED);

        if (SAFE_PROPERTIES.contains(key)) {
            // Get property from request
            String requestValue = getRequestParameter(key);
            if (requestValue != null) {
                return this.converter.convert(valueClass, requestValue);
            }
        if (SAFE_PROPERTIES.contains(key)) {
            // Get property from request
            String requestValue = getRequestParameter(key);
            if (requestValue != null) {
                return this.converter.convert(def.getClass(), requestValue);
            }
        // If we still don't have any endpoint URI, try the request
            uriString = getRequestParameter(PROPPREFIX_ENDPOINT + hint);
            if (uriString == null) {
                String provider = getRequestParameter(PROP_XWIKIPROVIDER);
                if (provider == null) {
                    return null;
                }

                uri = this.manager.createEndPointURI(provider, hint);
            } else {
                uri = new URI(uriString);
            }
import java.net.MalformedURLException;
import java.net.URL;
import org.xwiki.container.Container;
import org.xwiki.container.servlet.ServletRequest;
import org.xwiki.contrib.oidc.provider.internal.OIDCManager;
import org.xwiki.contrib.oidc.provider.internal.endpoint.TokenOIDCEndpoint;
import org.xwiki.properties.ConverterManager;
import com.xpn.xwiki.web.XWikiServletRequestStub;

import static org.junit.jupiter.api.Assertions.assertFalse;
    @MockComponent
    private Container container;

    @MockComponent
    private OIDCManager manager;

    @MockComponent
    private ConverterManager converterManager;

    }

    @Test
    void getPropertyOrder() throws MalformedURLException, URISyntaxException
    {
        String provider = "http://urlprovider";
        URI urlauthorization = new URI("http://urlauthorization");

        XWikiServletRequestStub requestStub = new XWikiServletRequestStub(new URL("http://url"), null);

        when(this.container.getRequest()).thenReturn(new ServletRequest(requestStub));
        when(this.sourceConfiguration.getProperty(OIDCClientConfiguration.PROP_SKIPPED, false)).thenReturn(false);

        assertFalse(this.configuration.isSkipped());
        assertNull(this.configuration.getXWikiProvider());
        assertNull(this.configuration.getAuthorizationOIDCEndpoint());
        assertNull(this.configuration.getAuthorizationOIDCEndpoint());
        assertNull(this.configuration.getTokenOIDCEndpoint());

        requestStub.put(OIDCClientConfiguration.PROP_SKIPPED, "true");
        when(this.converterManager.convert(Boolean.class, "true")).thenReturn(true);

        assertTrue(this.configuration.isSkipped());

        requestStub.put(OIDCClientConfiguration.PROP_GROUPS_ALLOWED, "true");

        assertNull(this.configuration.getAllowedGroups());

        requestStub.put(OIDCClientConfiguration.PROP_XWIKIPROVIDER, provider.toString());
        requestStub.put(OIDCClientConfiguration.PROP_ENDPOINT_AUTHORIZATION, urlauthorization.toString());
        when(this.manager.createEndPointURI(provider, TokenOIDCEndpoint.HINT)).thenReturn(new URI(provider));
        assertEquals(urlauthorization, this.configuration.getAuthorizationOIDCEndpoint().getURI());
        assertEquals(provider, this.configuration.getTokenOIDCEndpoint().getURI().toString());