Backported SQL Injection vulnerability fix and other small fixes

This commit is contained in:
Aurora Lahtela 2023-01-15 13:42:13 +02:00
parent d34172412c
commit 65e186f15c
6 changed files with 44 additions and 7 deletions

View File

@ -175,6 +175,11 @@ public class ResourceSvc implements ResourceService {
byte[] bytes = original.asBytes(); byte[] bytes = original.asBytes();
OpenOption[] overwrite = {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE}; OpenOption[] overwrite = {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE};
Path to = resourceSettings.getCustomizationDirectory().resolve(fileName); 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(); Path dir = to.getParent();
if (!Files.isSymbolicLink(dir)) Files.createDirectories(dir); if (!Files.isSymbolicLink(dir)) Files.createDirectories(dir);
Files.write(to, bytes, overwrite); Files.write(to, bytes, overwrite);

View File

@ -31,7 +31,11 @@ public class JoinAddress {
} }
public String getAddress() { public String getAddress() {
return address.get(); String joinAddress = address.get();
if (joinAddress.contains("\u0000")) {
joinAddress = joinAddress.split("\u0000", 2)[0];
}
return joinAddress;
} }
@Override @Override

View File

@ -21,6 +21,7 @@ import com.djrapitops.plan.identification.ServerUUID;
import java.sql.PreparedStatement; import java.sql.PreparedStatement;
import java.sql.SQLException; import java.sql.SQLException;
import java.sql.Types; import java.sql.Types;
import java.util.Collection;
import java.util.UUID; import java.util.UUID;
public class QueryParameterSetter { public class QueryParameterSetter {
@ -30,20 +31,36 @@ public class QueryParameterSetter {
public static void setParameters(PreparedStatement statement, Object... parameters) throws SQLException { public static void setParameters(PreparedStatement statement, Object... parameters) throws SQLException {
int index = 1; int index = 1;
for (Object parameter : parameters) { for (Object parameter : parameters) {
setParameter(statement, index, parameter); if (parameter instanceof Object[]) {
index++; 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 { private static void setParameter(PreparedStatement statement, int index, Object parameter) throws SQLException {
if (parameter == null) { if (parameter == null) {
statement.setNull(index, Types.VARCHAR); statement.setNull(index, Types.VARCHAR);
} else if (parameter instanceof Boolean) {
statement.setBoolean(index, (Boolean) parameter);
} else if (parameter instanceof Integer) { } else if (parameter instanceof Integer) {
statement.setInt(index, (Integer) parameter); statement.setInt(index, (Integer) parameter);
} else if (parameter instanceof Long) { } else if (parameter instanceof Long) {
statement.setLong(index, (Long) parameter); statement.setLong(index, (Long) parameter);
} else if (parameter instanceof Double) { } else if (parameter instanceof Double) {
statement.setDouble(index, (Double) parameter); statement.setDouble(index, (Double) parameter);
} else if (parameter instanceof Character) {
statement.setString(index, String.valueOf(parameter));
} else if (parameter instanceof Float) { } else if (parameter instanceof Float) {
statement.setFloat(index, (Float) parameter); statement.setFloat(index, (Float) parameter);
} else if (parameter instanceof String) { } else if (parameter instanceof String) {

View File

@ -21,6 +21,7 @@ import com.djrapitops.plan.identification.ServerUUID;
import com.djrapitops.plan.storage.database.queries.Query; import com.djrapitops.plan.storage.database.queries.Query;
import com.djrapitops.plan.storage.database.queries.QueryAllStatement; import com.djrapitops.plan.storage.database.queries.QueryAllStatement;
import com.djrapitops.plan.storage.database.queries.RowExtractors; 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.GeoInfoTable;
import com.djrapitops.plan.storage.database.sql.tables.ServerTable; import com.djrapitops.plan.storage.database.sql.tables.ServerTable;
import com.djrapitops.plan.storage.database.sql.tables.UserInfoTable; import com.djrapitops.plan.storage.database.sql.tables.UserInfoTable;
@ -171,9 +172,7 @@ public class GeoInfoQueries {
FROM + GeoInfoTable.TABLE_NAME + " g" + FROM + GeoInfoTable.TABLE_NAME + " g" +
INNER_JOIN + UsersTable.TABLE_NAME + " u on u.id=g." + GeoInfoTable.USER_ID + INNER_JOIN + UsersTable.TABLE_NAME + " u on u.id=g." + GeoInfoTable.USER_ID +
WHERE + GeoInfoTable.GEOLOCATION + WHERE + GeoInfoTable.GEOLOCATION +
" IN ('" + " IN (" + Sql.nParameters(selected.size()) + ")";
new TextStringBuilder().appendWithSeparators(selected, "','") + return db -> db.querySet(sql, RowExtractors.getInt(UsersTable.ID), selected);
"')";
return db -> db.querySet(sql, RowExtractors.getInt(UsersTable.ID));
} }
} }

View File

@ -16,10 +16,13 @@
*/ */
package com.djrapitops.plan.storage.database.sql.building; package com.djrapitops.plan.storage.database.sql.building;
import org.apache.commons.text.TextStringBuilder;
import java.sql.PreparedStatement; import java.sql.PreparedStatement;
import java.sql.SQLException; import java.sql.SQLException;
import java.sql.Types; import java.sql.Types;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
/** /**
* Duplicate String reducing utility class for SQL language Strings. * 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 MAX = "MAX(";
private static final String VARCHAR = "varchar("; 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) { public static String varchar(int length) {
return VARCHAR + length + ')'; return VARCHAR + length + ')';
} }

View File

@ -161,6 +161,9 @@ public class PlanFiles implements SubSystem {
Path dir = config.get().getResourceSettings().getCustomizationDirectory(); Path dir = config.get().getResourceSettings().getCustomizationDirectory();
if (dir.toFile().exists() && dir.toFile().isDirectory()) { if (dir.toFile().exists() && dir.toFile().isDirectory()) {
Path asPath = dir.resolve(resourceName); Path asPath = dir.resolve(resourceName);
if (!asPath.startsWith(dir)) {
return Optional.empty();
}
File found = asPath.toFile(); File found = asPath.toFile();
if (found.exists()) { if (found.exists()) {
return Optional.of(found); return Optional.of(found);