Skip to content

Commit 8151179

Browse files
ahornaceVladimir Kotal
ahornace
authored and
Vladimir Kotal
committed
Prevent XMLDecoder from loading other than whitelisted classes
1 parent c3cfd04 commit 8151179

File tree

5 files changed

+310
-3
lines changed

5 files changed

+310
-3
lines changed

‎opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1418,7 +1418,8 @@ private static Configuration decodeObject(InputStream in) throws IOException {
14181418
final LinkedList<Exception> exceptions = new LinkedList<>();
14191419
ExceptionListener listener = exceptions::addLast;
14201420

1421-
try (XMLDecoder d = new XMLDecoder(new BufferedInputStream(in), null, listener)) {
1421+
try (XMLDecoder d = new XMLDecoder(new BufferedInputStream(in), null, listener,
1422+
new ConfigurationClassLoader())) {
14221423
ret = d.readObject();
14231424
}
14241425

‎opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/ConfigurationClassLoader.java

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
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 org.opengrok.indexer.configuration;
24+
25+
import io.micrometer.statsd.StatsdFlavor;
26+
import org.opengrok.indexer.authorization.AuthControlFlag;
27+
import org.opengrok.indexer.authorization.AuthorizationEntity;
28+
import org.opengrok.indexer.authorization.AuthorizationPlugin;
29+
import org.opengrok.indexer.authorization.AuthorizationStack;
30+
import org.opengrok.indexer.authorization.IAuthorizationPlugin;
31+
import org.opengrok.indexer.configuration.Configuration.RemoteSCM;
32+
import org.opengrok.indexer.history.RepositoryInfo;
33+
34+
import java.beans.XMLDecoder;
35+
import java.util.ArrayList;
36+
import java.util.Collections;
37+
import java.util.HashMap;
38+
import java.util.Set;
39+
import java.util.TreeSet;
40+
import java.util.stream.Collectors;
41+
42+
/**
43+
* Temporary hack to prevent {@link XMLDecoder} to deserialize other than allowed classes. This tries to prevent
44+
* calling of methods on {@link ProcessBuilder} or {@link Runtime} (or similar) which could be used for code execution.
45+
*/
46+
public class ConfigurationClassLoader extends ClassLoader {
47+
48+
private static final Set<String> allowedClasses = Set.of(
49+
ArrayList.class,
50+
AuthControlFlag.class,
51+
AuthorizationEntity.class,
52+
AuthorizationPlugin.class,
53+
AuthorizationStack.class,
54+
Collections.class,
55+
Configuration.class,
56+
Filter.class,
57+
Group.class,
58+
HashMap.class,
59+
IAuthorizationPlugin.class,
60+
IgnoredNames.class,
61+
LuceneLockName.class,
62+
Project.class,
63+
RemoteSCM.class,
64+
RepositoryInfo.class,
65+
StatsdConfig.class,
66+
StatsdFlavor.class,
67+
String.class,
68+
SuggesterConfig.class,
69+
TreeSet.class,
70+
XMLDecoder.class
71+
).stream().map(Class::getName).collect(Collectors.toSet());
72+
73+
@Override
74+
public Class<?> loadClass(final String name) throws ClassNotFoundException {
75+
if (!allowedClasses.contains(name)) {
76+
throw new IllegalAccessError(name + " is not allowed to be used in configuration");
77+
}
78+
79+
return getClass().getClassLoader().loadClass(name);
80+
}
81+
}

‎opengrok-indexer/src/test/java/org/opengrok/indexer/configuration/ConfigurationTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,22 @@
2626
import java.beans.XMLDecoder;
2727
import java.beans.XMLEncoder;
2828
import java.io.BufferedInputStream;
29+
import java.io.BufferedReader;
2930
import java.io.ByteArrayInputStream;
3031
import java.io.ByteArrayOutputStream;
3132
import java.io.IOException;
33+
import java.io.InputStreamReader;
3234
import java.lang.reflect.Field;
3335
import java.lang.reflect.Modifier;
3436
import java.util.LinkedList;
37+
import java.util.stream.Collectors;
3538
import javax.xml.XMLConstants;
3639
import javax.xml.parsers.SAXParser;
3740
import javax.xml.parsers.SAXParserFactory;
41+
3842
import org.junit.jupiter.api.Test;
43+
import org.junit.jupiter.params.ParameterizedTest;
44+
import org.junit.jupiter.params.provider.ValueSource;
3945
import org.opengrok.indexer.util.ClassUtil;
4046

4147
import org.xml.sax.Attributes;
@@ -44,6 +50,7 @@
4450
import static org.junit.jupiter.api.Assertions.assertEquals;
4551
import static org.junit.jupiter.api.Assertions.assertNotEquals;
4652
import static org.junit.jupiter.api.Assertions.assertNotNull;
53+
import static org.junit.jupiter.api.Assertions.assertThrows;
4754

4855
/**
4956
*
@@ -168,4 +175,62 @@ public void serializationOrderTest() throws IOException {
168175
assertEquals(oldCfg.getGroups(), cfg.getGroups());
169176
}
170177
}
178+
179+
@ParameterizedTest
180+
@ValueSource(strings = {
181+
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
182+
"<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
183+
" <object class=\"java.lang.Runtime\" method=\"getRuntime\">\n" +
184+
" <void method=\"exec\">\n" +
185+
" <array class=\"java.lang.String\" length=\"2\">\n" +
186+
" <void index=\"0\">\n" +
187+
" <string>/usr/bin/nc</string>\n" +
188+
" </void>\n" +
189+
" <void index=\"1\">\n" +
190+
" <string>-l</string>\n" +
191+
" </void>\n" +
192+
" </array>\n" +
193+
" </void>\n" +
194+
" </object>\n" +
195+
"</java>",
196+
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
197+
"<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
198+
" <object class=\"java.lang.ProcessBuilder\">\n" +
199+
" <array class=\"java.lang.String\" length=\"1\" >\n" +
200+
" <void index=\"0\"> \n" +
201+
" <string>/usr/bin/curl https://oracle.com</string>\n" +
202+
" </void>\n" +
203+
" </array>\n" +
204+
" <void method=\"start\"/>\n" +
205+
" </object>\n" +
206+
"</java>",
207+
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
208+
"<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
209+
" <object class = \"java.io.FileOutputStream\"> \n" +
210+
" <string>opengrok_test.txt</string>\n" +
211+
" <method name = \"write\">\n" +
212+
" <array class=\"byte\" length=\"3\">\n" +
213+
" <void index=\"0\"><byte>96</byte></void>\n" +
214+
" <void index=\"1\"><byte>96</byte></void>\n" +
215+
" <void index=\"2\"><byte>96</byte></void>\n" +
216+
" </array>\n" +
217+
" </method>\n" +
218+
" <method name=\"close\"/>\n" +
219+
" </object>\n" +
220+
"</java>"
221+
})
222+
void testDeserializationOfNotWhiteListedClassThrowsError(final String exploit) {
223+
assertThrows(IllegalAccessError.class, () -> Configuration.makeXMLStringAsConfiguration(exploit));
224+
}
225+
226+
@Test
227+
void testLoadingValidConfiguration() throws IOException {
228+
try (var bf = new BufferedReader(new InputStreamReader(ConfigurationTest.class.getClassLoader()
229+
.getResourceAsStream("configuration/valid_configuration.xml")))) {
230+
String xml = bf.lines().collect(Collectors.joining(System.lineSeparator()));
231+
var config = Configuration.makeXMLStringAsConfiguration(xml);
232+
assertEquals("/opt/opengrok_data", config.getDataRoot());
233+
}
234+
}
235+
171236
}

‎opengrok-indexer/src/test/java/org/opengrok/indexer/configuration/RuntimeEnvironmentTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ public void testAuthorizationFlagDecodeInvalid() throws IOException {
734734
* @throws IOException I/O exception
735735
*/
736736
@Test
737-
public void testAuthorizationDecodeInvalid() throws IOException {
737+
void testAuthorizationDecodeInvalid() {
738738
String confString = "<?xml version='1.0' encoding='UTF-8'?>\n"
739739
+ "<java class=\"java.beans.XMLDecoder\" version=\"1.8.0_121\">\n"
740740
+ " <object class=\"org.opengrok.indexer.configuration.Configuration\">\n"
@@ -752,7 +752,7 @@ public void testAuthorizationDecodeInvalid() throws IOException {
752752
+ "\t</void>\n"
753753
+ " </object>\n"
754754
+ "</java>";
755-
assertThrows(IOException.class, () -> Configuration.makeXMLStringAsConfiguration(confString));
755+
assertThrows(IllegalAccessError.class, () -> Configuration.makeXMLStringAsConfiguration(confString));
756756
}
757757

758758
@Test

‎opengrok-indexer/src/test/resources/configuration/valid_configuration.xml

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<java version="11.0.8" class="java.beans.XMLDecoder">
3+
<object class="org.opengrok.indexer.configuration.Configuration" id="Configuration0">
4+
<void property="ctags">
5+
<string>/usr/local/bin/ctags</string>
6+
</void>
7+
<void property="dataRoot">
8+
<string>/opt/opengrok_data</string>
9+
</void>
10+
<void id="IgnoredNames0" property="ignoredNames">
11+
<void id="IgnoredDirs0" property="ignoredDirs">
12+
<void property="items">
13+
<void method="add">
14+
<string>.git</string>
15+
</void>
16+
<void method="add">
17+
<string>.hg</string>
18+
</void>
19+
<void method="add">
20+
<string>.repo</string>
21+
</void>
22+
<void method="add">
23+
<string>.bk</string>
24+
</void>
25+
<void method="add">
26+
<string>.bzr</string>
27+
</void>
28+
<void method="add">
29+
<string>.svn</string>
30+
</void>
31+
<void method="add">
32+
<string>SCCS</string>
33+
</void>
34+
<void method="add">
35+
<string>.razor</string>
36+
</void>
37+
<void method="add">
38+
<string>RCS</string>
39+
</void>
40+
<void method="add">
41+
<string>CVS</string>
42+
</void>
43+
<void method="add">
44+
<string>CVSROOT</string>
45+
</void>
46+
</void>
47+
</void>
48+
<void id="IgnoredFiles0" property="ignoredFiles">
49+
<void property="items">
50+
<void method="add">
51+
<string>.git</string>
52+
</void>
53+
<void method="add">
54+
<string>.hgtags</string>
55+
</void>
56+
<void method="add">
57+
<string>.hgignore</string>
58+
</void>
59+
<void method="add">
60+
<string>.cvsignore</string>
61+
</void>
62+
<void method="add">
63+
<string>.p4config</string>
64+
</void>
65+
</void>
66+
</void>
67+
</void>
68+
<void property="projects">
69+
<void method="put">
70+
<string>opengrok</string>
71+
<object class="org.opengrok.indexer.configuration.Project">
72+
<void property="historyEnabled">
73+
<boolean>true</boolean>
74+
</void>
75+
<void property="indexed">
76+
<boolean>true</boolean>
77+
</void>
78+
<void property="mergeCommitsEnabled">
79+
<boolean>true</boolean>
80+
</void>
81+
<void property="name">
82+
<string>opengrok</string>
83+
</void>
84+
<void property="path">
85+
<string>/opengrok</string>
86+
</void>
87+
</object>
88+
</void>
89+
<void method="put">
90+
<string>cuda-samples</string>
91+
<object class="org.opengrok.indexer.configuration.Project">
92+
<void property="historyEnabled">
93+
<boolean>true</boolean>
94+
</void>
95+
<void property="indexed">
96+
<boolean>true</boolean>
97+
</void>
98+
<void property="mergeCommitsEnabled">
99+
<boolean>true</boolean>
100+
</void>
101+
<void property="name">
102+
<string>cuda-samples</string>
103+
</void>
104+
<void property="path">
105+
<string>/cuda-samples</string>
106+
</void>
107+
</object>
108+
</void>
109+
<void method="put">
110+
<string>CudaSift</string>
111+
<object class="org.opengrok.indexer.configuration.Project">
112+
<void property="historyEnabled">
113+
<boolean>true</boolean>
114+
</void>
115+
<void property="indexed">
116+
<boolean>true</boolean>
117+
</void>
118+
<void property="mergeCommitsEnabled">
119+
<boolean>true</boolean>
120+
</void>
121+
<void property="name">
122+
<string>CudaSift</string>
123+
</void>
124+
<void property="path">
125+
<string>/CudaSift</string>
126+
</void>
127+
</object>
128+
</void>
129+
</void>
130+
<void property="projectsEnabled">
131+
<boolean>true</boolean>
132+
</void>
133+
<void property="sourceRoot">
134+
<string>/opt/opengrok_src</string>
135+
</void>
136+
<void id="SuggesterConfig0" property="suggesterConfig">
137+
<void property="allowedFields">
138+
<void method="clear"/>
139+
<void method="add">
140+
<string>defs</string>
141+
</void>
142+
<void method="add">
143+
<string>path</string>
144+
</void>
145+
<void method="add">
146+
<string>hist</string>
147+
</void>
148+
<void method="add">
149+
<string>refs</string>
150+
</void>
151+
<void method="add">
152+
<string>type</string>
153+
</void>
154+
<void method="add">
155+
<string>full</string>
156+
</void>
157+
</void>
158+
</void>
159+
</object>
160+
</java>

0 commit comments

Comments
 (0)