Skip to content

Commit 8d26ce8

Browse files
author
Vladimir Kotal
authored
sanitize class paths (#3781)
* sanitize class paths * rename parameter to fit the camel case * another camel case * more variable renaming * add file separator * add warnings
1 parent 912a7af commit 8d26ce8

File tree

1 file changed

+39
-16
lines changed

1 file changed

+39
-16
lines changed

opengrok-indexer/src/main/java/org/opengrok/indexer/framework/PluginFramework.java

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
import java.util.jar.JarFile;
3737
import java.util.logging.Level;
3838
import java.util.logging.Logger;
39+
40+
import org.jetbrains.annotations.Nullable;
3941
import org.opengrok.indexer.authorization.IAuthorizationPlugin;
4042
import org.opengrok.indexer.logger.LoggerFactory;
4143
import org.opengrok.indexer.util.IOUtils;
@@ -49,6 +51,8 @@ public abstract class PluginFramework<PluginType> {
4951

5052
private static final Logger LOGGER = LoggerFactory.getLogger(PluginFramework.class);
5153

54+
private static final String CLASS_SUFFIX = ".class";
55+
5256
/**
5357
* Class of the plugin type, necessary for instantiating and searching.
5458
*/
@@ -248,16 +252,16 @@ protected List<Class<?>> getSuperclassesAndInterfaces(Class<?> clazz) {
248252
* delegates the loading to the custom class loader
249253
* {@link #loadClass(String)}.
250254
*
251-
* @param classfiles list of files which possibly contain a java class
255+
* @param fileList list of files which possibly contain a java class
252256
* @see #handleLoadClass(String)
253257
* @see #loadClass(String)
254258
*/
255-
private void loadClassFiles(List<File> classfiles) {
259+
private void loadClassFiles(List<File> fileList) {
256260
PluginType plugin;
257261

258-
for (File file : classfiles) {
262+
for (File file : fileList) {
259263
String classname = getClassName(file);
260-
if (classname.isEmpty()) {
264+
if (classname == null || classname.isEmpty()) {
261265
continue;
262266
}
263267
// Load the class in memory and try to find a configured space for this class.
@@ -274,23 +278,20 @@ private void loadClassFiles(List<File> classfiles) {
274278
* delegates the loading to the custom class loader
275279
* {@link #loadClass(String)}.
276280
*
277-
* @param jarfiles list of jar files containing java classes
281+
* @param fileList list of jar files containing java classes
278282
* @see #handleLoadClass(String)
279283
* @see #loadClass(String)
280284
*/
281-
private void loadJarFiles(List<File> jarfiles) {
285+
private void loadJarFiles(List<File> fileList) {
282286
PluginType pf;
283287

284-
for (File file : jarfiles) {
288+
for (File file : fileList) {
285289
try (JarFile jar = new JarFile(file)) {
286290
Enumeration<JarEntry> entries = jar.entries();
287291
while (entries.hasMoreElements()) {
288292
JarEntry entry = entries.nextElement();
289-
if (!entry.getName().endsWith(".class")) {
290-
continue;
291-
}
292293
String classname = getClassName(entry);
293-
if (!entry.getName().endsWith(".class") || classname.isEmpty()) {
294+
if (classname == null || classname.isEmpty()) {
294295
continue;
295296
}
296297
// Load the class in memory and try to find a configured space for this class.
@@ -304,18 +305,40 @@ private void loadJarFiles(List<File> jarfiles) {
304305
}
305306
}
306307

307-
private String getClassName(File f) {
308-
String classname = f.getAbsolutePath().substring(pluginDirectory.getAbsolutePath().length() + 1);
308+
@Nullable
309+
private String getClassName(File file) {
310+
if (!file.getName().endsWith(CLASS_SUFFIX)) {
311+
return null;
312+
}
313+
314+
String classname = file.getAbsolutePath().substring(pluginDirectory.getAbsolutePath().length() + 1);
309315
classname = classname.replace(File.separatorChar, '.'); // convert to package name
310316
// no need to check for the index from lastIndexOf because we're in a branch
311317
// where we expect the .class suffix
312318
classname = classname.substring(0, classname.lastIndexOf('.')); // strip .class
313319
return classname;
314320
}
315321

316-
private String getClassName(JarEntry f) {
322+
@Nullable
323+
private String getClassName(JarEntry jarEntry) {
324+
final String filePath = jarEntry.getName();
325+
if (!filePath.endsWith(CLASS_SUFFIX)) {
326+
return null;
327+
}
328+
329+
File file = new File(pluginDirectory.getAbsolutePath(), filePath);
330+
try {
331+
if (!file.getCanonicalPath().startsWith(pluginDirectory.getCanonicalPath() + File.separator)) {
332+
LOGGER.log(Level.WARNING, "canonical path for jar entry {0} leads outside the origin", filePath);
333+
return null;
334+
}
335+
} catch (IOException e) {
336+
LOGGER.log(Level.WARNING, "failed to get canonical path for {0}", file);
337+
return null;
338+
}
339+
317340
// java jar always uses / as separator
318-
String classname = f.getName().replace('/', '.'); // convert to package name
341+
String classname = filePath.replace('/', '.'); // convert to package name
319342
return classname.substring(0, classname.lastIndexOf('.')); // strip .class
320343
}
321344

@@ -364,7 +387,7 @@ public final void reload() {
364387

365388
// load all other possible plugin classes.
366389
if (isLoadClassesEnabled()) {
367-
loadClassFiles(IOUtils.listFilesRec(pluginDirectory, ".class"));
390+
loadClassFiles(IOUtils.listFilesRec(pluginDirectory, CLASS_SUFFIX));
368391
}
369392
if (isLoadJarsEnabled()) {
370393
loadJarFiles(IOUtils.listFiles(pluginDirectory, ".jar"));

0 commit comments

Comments
 (0)