Backported SQL Injection vulnerability fix and other small fixes

DeerGiteaMirror · Jan 15, 2023 · 65e186f · 65e186f
1 parent d341724
commit 65e186f
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 7 deletions.
diff --git a/Plan/common/src/main/java/com/djrapitops/plan/delivery/web/ResourceSvc.java b/Plan/common/src/main/java/com/djrapitops/plan/delivery/web/ResourceSvc.java
@@ -175,6 +175,11 @@ public WebResource writeCustomized(String fileName, Supplier<WebResource> source
         byte[] bytes = original.asBytes();
         OpenOption[] overwrite = {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE};
         Path to = resourceSettings.getCustomizationDirectory().resolve(fileName);
+        if (!to.startsWith(resourceSettings.getCustomizationDirectory())) {
+            throw new IllegalArgumentException(
+                    "Absolute path was given for writing a customized file, " +
+                            "writing outside customization directory is prevented for security reasons.");
+        }
         Path dir = to.getParent();
         if (!Files.isSymbolicLink(dir)) Files.createDirectories(dir);
         Files.write(to, bytes, overwrite);

diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/event/JoinAddress.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/event/JoinAddress.java
@@ -31,7 +31,11 @@ public JoinAddress(Supplier<String> address) {
     }
 
     public String getAddress() {
-        return address.get();
+        String joinAddress = address.get();
+        if (joinAddress.contains("\u0000")) {
+            joinAddress = joinAddress.split("\u0000", 2)[0];
+        }
+        return joinAddress;
     }
 
     @Override

diff --git a/...mmon/src/main/java/com/djrapitops/plan/storage/database/queries/QueryParameterSetter.java b/...mmon/src/main/java/com/djrapitops/plan/storage/database/queries/QueryParameterSetter.java
@@ -21,6 +21,7 @@
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.sql.Types;
+import java.util.Collection;
 import java.util.UUID;
 
 public class QueryParameterSetter {
@@ -30,20 +31,36 @@ private QueryParameterSetter() {}
     public static void setParameters(PreparedStatement statement, Object... parameters) throws SQLException {
         int index = 1;
         for (Object parameter : parameters) {
-            setParameter(statement, index, parameter);
-            index++;
+            if (parameter instanceof Object[]) {
+                for (Object arrayParameter : (Object[]) parameter) {
+                    setParameter(statement, index, arrayParameter);
+                    index++;
+                }
+            } else if (parameter instanceof Collection) {
+                for (Object collectionParameter : (Collection<?>) parameter) {
+                    setParameter(statement, index, collectionParameter);
+                    index++;
+                }
+            } else {
+                setParameter(statement, index, parameter);
+                index++;
+            }
         }
     }
 
     private static void setParameter(PreparedStatement statement, int index, Object parameter) throws SQLException {
         if (parameter == null) {
             statement.setNull(index, Types.VARCHAR);
+        } else if (parameter instanceof Boolean) {
+            statement.setBoolean(index, (Boolean) parameter);
         } else if (parameter instanceof Integer) {
             statement.setInt(index, (Integer) parameter);
         } else if (parameter instanceof Long) {
             statement.setLong(index, (Long) parameter);
         } else if (parameter instanceof Double) {
             statement.setDouble(index, (Double) parameter);
+        } else if (parameter instanceof Character) {
+            statement.setString(index, String.valueOf(parameter));
         } else if (parameter instanceof Float) {
             statement.setFloat(index, (Float) parameter);
         } else if (parameter instanceof String) {

diff --git a/...on/src/main/java/com/djrapitops/plan/storage/database/queries/objects/GeoInfoQueries.java b/...on/src/main/java/com/djrapitops/plan/storage/database/queries/objects/GeoInfoQueries.java
@@ -21,6 +21,7 @@
 import com.djrapitops.plan.storage.database.queries.Query;
 import com.djrapitops.plan.storage.database.queries.QueryAllStatement;
 import com.djrapitops.plan.storage.database.queries.RowExtractors;
+import com.djrapitops.plan.storage.database.sql.building.Sql;
 import com.djrapitops.plan.storage.database.sql.tables.GeoInfoTable;
 import com.djrapitops.plan.storage.database.sql.tables.ServerTable;
 import com.djrapitops.plan.storage.database.sql.tables.UserInfoTable;
@@ -171,9 +172,7 @@ public static Query<Set<Integer>> userIdsOfPlayersWithGeolocations(List<String>
                 FROM + GeoInfoTable.TABLE_NAME + " g" +
                 INNER_JOIN + UsersTable.TABLE_NAME + " u on u.id=g." + GeoInfoTable.USER_ID +
                 WHERE + GeoInfoTable.GEOLOCATION +
-                " IN ('" +
-                new TextStringBuilder().appendWithSeparators(selected, "','") +
-                "')";
-        return db -> db.querySet(sql, RowExtractors.getInt(UsersTable.ID));
+                " IN (" + Sql.nParameters(selected.size()) + ")";
+        return db -> db.querySet(sql, RowExtractors.getInt(UsersTable.ID), selected);
     }
 }
diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/building/Sql.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/database/sql/building/Sql.java
@@ -16,10 +16,13 @@
  */
 package com.djrapitops.plan.storage.database.sql.building;
 
+import org.apache.commons.text.TextStringBuilder;
+
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.sql.Types;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
 
 /**
  * Duplicate String reducing utility class for SQL language Strings.
@@ -56,6 +59,12 @@ public abstract class Sql {
     private static final String MAX = "MAX(";
     private static final String VARCHAR = "varchar(";
 
+    public static String nParameters(int n) {
+        return new TextStringBuilder()
+                .appendWithSeparators(IntStream.range(0, n).mapToObj(i -> "?").iterator(), ",")
+                .toString();
+    }
+
     public static String varchar(int length) {
         return VARCHAR + length + ')';
     }

diff --git a/Plan/common/src/main/java/com/djrapitops/plan/storage/file/PlanFiles.java b/Plan/common/src/main/java/com/djrapitops/plan/storage/file/PlanFiles.java
@@ -161,6 +161,9 @@ public Optional<File> attemptToFind(String resourceName) {
         Path dir = config.get().getResourceSettings().getCustomizationDirectory();
         if (dir.toFile().exists() && dir.toFile().isDirectory()) {
             Path asPath = dir.resolve(resourceName);
+            if (!asPath.startsWith(dir)) {
+                return Optional.empty();
+            }
             File found = asPath.toFile();
             if (found.exists()) {
                 return Optional.of(found);