Skip to content

Commit 487154b

Browse files
author
Vladimir Kotal
committed
restrict XML decoding for plugin Configuration
1 parent a559414 commit 487154b

File tree

4 files changed

+164
-1
lines changed

4 files changed

+164
-1
lines changed

‎plugins/src/main/java/opengrok/auth/plugin/configuration/Configuration.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,8 @@ public static Configuration makeXMLStringAsConfiguration(String xmlconfig) throw
161161
private static Configuration decodeObject(InputStream in) throws IOException {
162162
final Object ret;
163163

164-
try (XMLDecoder d = new XMLDecoder(new BufferedInputStream(in), null, null, Configuration.class.getClassLoader())) {
164+
try (XMLDecoder d = new XMLDecoder(new BufferedInputStream(in), null, null,
165+
new PluginConfigurationClassLoader())) {
165166
ret = d.readObject();
166167
}
167168

‎plugins/src/main/java/opengrok/auth/plugin/configuration/PluginConfigurationClassLoader.java

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* CDDL HEADER START
3+
*
4+
* The contents of this file are subject to the terms of the
5+
* Common Development and Distribution License (the "License").
6+
* You may not use this file except in compliance with the License.
7+
*
8+
* See LICENSE.txt included in this distribution for the specific
9+
* language governing permissions and limitations under the License.
10+
*
11+
* When distributing Covered Code, include this CDDL HEADER in each
12+
* file and include the License file at LICENSE.txt.
13+
* If applicable, add the following below this CDDL HEADER, with the
14+
* fields enclosed by brackets "[]" replaced with your own identifying
15+
* information: Portions Copyright [yyyy] [name of copyright owner]
16+
*
17+
* CDDL HEADER END
18+
*/
19+
20+
/*
21+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
22+
*/
23+
package opengrok.auth.plugin.configuration;
24+
25+
import opengrok.auth.plugin.ldap.LdapServer;
26+
27+
import java.beans.XMLDecoder;
28+
import java.util.ArrayList;
29+
import java.util.Collections;
30+
import java.util.Date;
31+
import java.util.HashMap;
32+
import java.util.Set;
33+
import java.util.TreeSet;
34+
import java.util.stream.Collectors;
35+
36+
/**
37+
* Temporary hack to prevent {@link XMLDecoder} to deserialize other than allowed classes. This tries to prevent
38+
* calling of methods on {@link ProcessBuilder} or {@link Runtime} (or similar) which could be used for code execution.
39+
*/
40+
public class PluginConfigurationClassLoader extends ClassLoader {
41+
42+
private static final Set<String> allowedClasses = Set.of(
43+
ArrayList.class,
44+
Collections.class,
45+
Configuration.class,
46+
Date.class,
47+
HashMap.class,
48+
LdapServer.class,
49+
String.class,
50+
TreeSet.class,
51+
XMLDecoder.class
52+
).stream().map(Class::getName).collect(Collectors.toSet());
53+
54+
@Override
55+
public Class<?> loadClass(final String name) throws ClassNotFoundException {
56+
if (!allowedClasses.contains(name)) {
57+
throw new IllegalAccessError(name + " is not allowed to be used in configuration");
58+
}
59+
60+
return getClass().getClassLoader().loadClass(name);
61+
}
62+
}

‎plugins/src/test/java/opengrok/auth/plugin/configuration/ConfigurationTest.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import java.beans.XMLEncoder;
2828
import java.io.ByteArrayInputStream;
2929
import java.io.ByteArrayOutputStream;
30+
import java.io.File;
31+
import java.io.IOException;
3032
import java.util.ArrayList;
3133
import java.util.LinkedList;
3234
import java.util.List;
@@ -36,9 +38,12 @@
3638
import opengrok.auth.plugin.util.WebHook;
3739
import opengrok.auth.plugin.util.WebHooks;
3840
import org.junit.jupiter.api.Test;
41+
import org.junit.jupiter.params.ParameterizedTest;
42+
import org.junit.jupiter.params.provider.ValueSource;
3943

4044
import static org.junit.jupiter.api.Assertions.assertEquals;
4145
import static org.junit.jupiter.api.Assertions.assertNotNull;
46+
import static org.junit.jupiter.api.Assertions.assertThrows;
4247

4348
/**
4449
*
@@ -99,4 +104,60 @@ public void testEncodeDecode() {
99104
throw afe;
100105
}
101106
}
107+
108+
@ParameterizedTest
109+
@ValueSource(strings = {
110+
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
111+
"<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
112+
" <object class=\"java.lang.Runtime\" method=\"getRuntime\">\n" +
113+
" <void method=\"exec\">\n" +
114+
" <array class=\"java.lang.String\" length=\"2\">\n" +
115+
" <void index=\"0\">\n" +
116+
" <string>/usr/bin/nc</string>\n" +
117+
" </void>\n" +
118+
" <void index=\"1\">\n" +
119+
" <string>-l</string>\n" +
120+
" </void>\n" +
121+
" </array>\n" +
122+
" </void>\n" +
123+
" </object>\n" +
124+
"</java>",
125+
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
126+
"<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
127+
" <object class=\"java.lang.ProcessBuilder\">\n" +
128+
" <array class=\"java.lang.String\" length=\"1\" >\n" +
129+
" <void index=\"0\"> \n" +
130+
" <string>/usr/bin/curl https://oracle.com</string>\n" +
131+
" </void>\n" +
132+
" </array>\n" +
133+
" <void method=\"start\"/>\n" +
134+
" </object>\n" +
135+
"</java>",
136+
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
137+
"<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
138+
" <object class = \"java.io.FileOutputStream\"> \n" +
139+
" <string>opengrok_test.txt</string>\n" +
140+
" <method name = \"write\">\n" +
141+
" <array class=\"byte\" length=\"3\">\n" +
142+
" <void index=\"0\"><byte>96</byte></void>\n" +
143+
" <void index=\"1\"><byte>96</byte></void>\n" +
144+
" <void index=\"2\"><byte>96</byte></void>\n" +
145+
" </array>\n" +
146+
" </method>\n" +
147+
" <method name=\"close\"/>\n" +
148+
" </object>\n" +
149+
"</java>"
150+
})
151+
void testDeserializationOfNotWhiteListedClassThrowsError(final String exploit) {
152+
assertThrows(IllegalAccessError.class, () -> Configuration.makeXMLStringAsConfiguration(exploit));
153+
}
154+
155+
@Test
156+
void testReadCacheValid() throws IOException {
157+
File testFile = new File(ConfigurationTest.class.getClassLoader().
158+
getResource("opengrok/auth/plugin/configuration/plugin-config.xml").getFile());
159+
Configuration config = Configuration.read(testFile);
160+
assertNotNull(config);
161+
assertEquals(2, config.getServers().size());
162+
}
102163
}

‎plugins/src/test/resources/opengrok/auth/plugin/configuration/plugin-config.xml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<java version="1.8.0_65" class="java.beans.XMLDecoder">
3+
<object class="opengrok.auth.plugin.configuration.Configuration">
4+
<void property="interval">
5+
<int>900000</int>
6+
</void>
7+
<void property="searchTimeout">
8+
<int>500</int>
9+
</void>
10+
<void property="searchBase">
11+
<string>dc=foobar,dc=com</string>
12+
</void>
13+
<void property="connectTimeout">
14+
<int>5000</int>
15+
</void>
16+
<void property="countLimit">
17+
<int>100</int>
18+
</void>
19+
<void property="servers">
20+
<void method="add">
21+
<object class="opengrok.auth.plugin.ldap.LdapServer">
22+
<void property="name">
23+
<string>ldap://ldap1.foobar.com</string>
24+
</void>
25+
<void property="connectTimeout">
26+
<int>5000</int>
27+
</void>
28+
</object>
29+
</void>
30+
<void method="add">
31+
<object class="opengrok.auth.plugin.ldap.LdapServer">
32+
<void property="name">
33+
<string>ldap://ldap2.foobar.com</string>
34+
</void>
35+
</object>
36+
</void>
37+
</void>
38+
</object>
39+
</java>

0 commit comments

Comments
 (0)