- FIX: Fixed potential security vulnerability that allows directory t…

…raversal.

- FIX: Fixed potential security vulnerability that allows SQL injection.

build.gradle

@@ -21,7 +21,7 @@ apply plugin: 'com.bmuschko.cargo-base'
 compileJava.options.encoding = 'UTF-8'
 
 group 'uk.ac.hutton.germinate'
-version '4.1.3'
+version '4.1.4'
 
 sourceCompatibility = 1.8
 

src/main/java/jhi/germinate/server/resource/FilteredResource.java

@@ -82,7 +82,7 @@ default <T extends Record> void filter(SelectJoinStep<T> step, Filter[] filters,
 
 	default Condition filterIndividual(Filter filter, boolean jsonOperationAllowed)
 	{
-		Field<Object> field = DSL.field(filter.getSafeColumn());
+		Field<Object> field = DSL.field("{0}", filter.getSafeColumn());
 		List<String> values = new ArrayList<>();
 
 		if (!CollectionUtils.isEmpty(filter.getValues()))

src/main/java/jhi/germinate/server/resource/PaginatedServerResource.java

@@ -101,9 +101,9 @@ protected <T extends Record> SelectForUpdateStep<T> setPaginationAndOrderBy(Sele
 		if (ascending != null && orderBy != null)
 		{
 			if (ascending)
-				step.orderBy(DSL.field(orderBy).asc());
+				step.orderBy(DSL.field("{0}", orderBy).asc());
 			else
-				step.orderBy(DSL.field(orderBy).desc());
+				step.orderBy(DSL.field("{0}", orderBy).desc());
 		}
 
 		return step.limit(pageSize)

src/main/java/jhi/germinate/server/resource/images/ImageSourceResource.java

@@ -1,5 +1,6 @@
 package jhi.germinate.server.resource.images;
 
+import jhi.germinate.server.util.*;
 import net.coobird.thumbnailator.Thumbnails;
 
 import org.apache.commons.io.IOUtils;
@@ -13,7 +14,6 @@
 
 import jhi.germinate.resource.enums.ServerProperty;
 import jhi.germinate.server.auth.*;
-import jhi.germinate.server.util.StringUtils;
 import jhi.germinate.server.util.watcher.PropertyWatcher;
 
 /**
@@ -81,7 +81,12 @@ public void getImage()
 		}
 		else if (!StringUtils.isEmpty(name))
 		{
-			File large = new File(new File(new File(PropertyWatcher.get(ServerProperty.DATA_DIRECTORY_EXTERNAL), "images"), type.name()), name);
+			File parent = new File(PropertyWatcher.get(ServerProperty.DATA_DIRECTORY_EXTERNAL), "images");
+			File large = new File(new File(parent, type.name()), name);
+
+			if (!FileUtils.isSubDirectory(parent, large))
+				throw new ResourceException(Status.CLIENT_ERROR_FORBIDDEN);
+
 			File small = new File(large.getParentFile(), "thumbnail-" + large.getName());
 
 			name = large.getName();

src/main/java/jhi/germinate/server/resource/images/ImageSvgSourceResource.java

@@ -1,5 +1,6 @@
 package jhi.germinate.server.resource.images;
 
+import jhi.germinate.server.util.*;
 import org.apache.commons.io.IOUtils;
 import org.restlet.data.Status;
 import org.restlet.data.*;
@@ -10,7 +11,6 @@
 
 import jhi.germinate.resource.enums.ServerProperty;
 import jhi.germinate.server.auth.*;
-import jhi.germinate.server.util.StringUtils;
 import jhi.germinate.server.util.watcher.PropertyWatcher;
 
 /**
@@ -40,7 +40,12 @@ public void getImage()
 	{
 		if (!StringUtils.isEmpty(name))
 		{
-			File file = new File(new File(new File(PropertyWatcher.get(ServerProperty.DATA_DIRECTORY_EXTERNAL), "images"), "template"), name);
+			File parent = new File(new File(PropertyWatcher.get(ServerProperty.DATA_DIRECTORY_EXTERNAL), "images"), "template");
+			File file = new File(parent, name);
+
+			if (!FileUtils.isSubDirectory(parent, file))
+				throw new ResourceException(Status.CLIENT_ERROR_FORBIDDEN);
+
 
 			if (file.exists() && file.isFile())
 			{

src/main/java/jhi/germinate/server/resource/importers/DataImportRunner.java

@@ -43,7 +43,7 @@ public static List<AsyncExportResult> importData(String uuid)
 												 .where(DATA_IMPORT_JOBS.UUID.eq(uuid))
 												 .and(DATA_IMPORT_JOBS.STATUS.eq(DataImportJobsStatus.completed))
 												 .and(DATA_IMPORT_JOBS.IMPORTED.eq(false))
-												 .and(DSL.field("JSON_LENGTH(" + DATA_IMPORT_JOBS.FEEDBACK.getName() + ")").eq(0))
+												 .and(DSL.field("JSON_LENGTH({0})", DATA_IMPORT_JOBS.FEEDBACK.getName()).eq(0))
 												 .fetchAnyInto(DataImportJobsRecord.class);
 
 			if (record == null)

src/main/java/jhi/germinate/server/resource/stats/DatasetStatsResource.java

@@ -47,7 +47,7 @@ public FileRepresentation getJson()
 				Set<String> years = new TreeSet<>();
 				Map<String, DatasetStats> datasetTypeToStats = new TreeMap<>();
 
-				Field<String> theYear = DSL.field("IF(ISNULL(" + DATASETS.DATE_START.getName() + "), 'UNKNOWN', DATE_FORMAT({0}, {1}))", SQLDataType.VARCHAR, DATASETS.DATE_START, DSL.inline("%Y"));
+				Field<String> theYear = DSL.field("IF(ISNULL({2}), 'UNKNOWN', DATE_FORMAT({0}, {1}))", SQLDataType.VARCHAR, DATASETS.DATE_START, DSL.inline("%Y"), DATASETS.DATE_START.getName());
 
 				context.select(
 					DATASETTYPES.DESCRIPTION.as("dataset_type"),

src/main/java/jhi/germinate/server/util/FileUtils.java

@@ -0,0 +1,39 @@
+package jhi.germinate.server.util;
+
+import java.io.*;
+
+public class FileUtils
+{
+	/**
+	 * Checks, whether the child directory is a subdirectory of the base
+	 * directory.
+	 *
+	 * @param base  the base directory.
+	 * @param child the suspected child directory.
+	 * @return true, if the child is a subdirectory of the base directory.
+	 * @throws IOException if an IOError occured during the test.
+	 */
+	public static boolean isSubDirectory(File base, File child)
+	{
+		try
+		{
+			base = base.getCanonicalFile();
+			child = child.getCanonicalFile();
+		}
+		catch (IOException e)
+		{
+			return false;
+		}
+
+		File parentFile = child;
+		while (parentFile != null)
+		{
+			if (base.equals(parentFile))
+			{
+				return true;
+			}
+			parentFile = parentFile.getParentFile();
+		}
+		return false;
+	}
+}