diff --git a/Plan/bukkit/src/main/java/com/djrapitops/plan/gathering/listeners/bukkit/PlayerOnlineListener.java b/Plan/bukkit/src/main/java/com/djrapitops/plan/gathering/listeners/bukkit/PlayerOnlineListener.java index 373957556..1f6bb5473 100644 --- a/Plan/bukkit/src/main/java/com/djrapitops/plan/gathering/listeners/bukkit/PlayerOnlineListener.java +++ b/Plan/bukkit/src/main/java/com/djrapitops/plan/gathering/listeners/bukkit/PlayerOnlineListener.java @@ -16,6 +16,7 @@ */ package com.djrapitops.plan.gathering.listeners.bukkit; +import com.djrapitops.plan.gathering.JoinAddressValidator; import com.djrapitops.plan.gathering.cache.JoinAddressCache; import com.djrapitops.plan.gathering.domain.BukkitPlayerData; import com.djrapitops.plan.gathering.domain.event.PlayerJoin; @@ -29,6 +30,7 @@ import com.djrapitops.plan.storage.database.DBSystem; import com.djrapitops.plan.storage.database.transactions.events.BanStatusTransaction; import com.djrapitops.plan.storage.database.transactions.events.KickStoreTransaction; import com.djrapitops.plan.storage.database.transactions.events.StoreAllowlistBounceTransaction; +import com.djrapitops.plan.utilities.dev.Untrusted; import com.djrapitops.plan.utilities.logging.ErrorContext; import com.djrapitops.plan.utilities.logging.ErrorLogger; import org.bukkit.event.EventHandler; @@ -51,6 +53,7 @@ public class PlayerOnlineListener implements Listener { private final PlayerJoinEventConsumer playerJoinEventConsumer; private final PlayerLeaveEventConsumer playerLeaveEventConsumer; + private final JoinAddressValidator joinAddressValidator; private final JoinAddressCache joinAddressCache; private final ServerInfo serverInfo; @@ -62,6 +65,7 @@ public class PlayerOnlineListener implements Listener { public PlayerOnlineListener( PlayerJoinEventConsumer playerJoinEventConsumer, PlayerLeaveEventConsumer playerLeaveEventConsumer, + JoinAddressValidator joinAddressValidator, JoinAddressCache joinAddressCache, ServerInfo serverInfo, DBSystem dbSystem, @@ -70,6 +74,7 @@ public class PlayerOnlineListener implements Listener { ) { this.playerJoinEventConsumer = playerJoinEventConsumer; this.playerLeaveEventConsumer = playerLeaveEventConsumer; + this.joinAddressValidator = joinAddressValidator; this.joinAddressCache = joinAddressCache; this.serverInfo = serverInfo; this.dbSystem = dbSystem; @@ -89,17 +94,8 @@ public class PlayerOnlineListener implements Listener { dbSystem.getDatabase().executeTransaction(new StoreAllowlistBounceTransaction(playerUUID, event.getPlayer().getName(), serverUUID, System.currentTimeMillis())); } - String address = event.getHostname(); - if (!address.isEmpty()) { - if (address.contains(":")) { - address = address.substring(0, address.lastIndexOf(':')); - } - if (address.contains("\u0000")) { - address = address.substring(0, address.indexOf('\u0000')); - } - if (address.contains("fml")) { - address = address.substring(0, address.lastIndexOf("fml")); - } + @Untrusted String address = joinAddressValidator.sanitize(event.getHostname()); + if (joinAddressValidator.isValid(address)) { joinAddressCache.put(playerUUID, address); } dbSystem.getDatabase().executeTransaction(new BanStatusTransaction(playerUUID, serverUUID, banned)); diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/JoinAddressValidator.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/JoinAddressValidator.java new file mode 100644 index 000000000..229c5bfb8 --- /dev/null +++ b/Plan/common/src/main/java/com/djrapitops/plan/gathering/JoinAddressValidator.java @@ -0,0 +1,80 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan.gathering; + +import com.djrapitops.plan.settings.config.PlanConfig; +import com.djrapitops.plan.settings.config.paths.DataGatheringSettings; +import com.djrapitops.plan.utilities.dev.Untrusted; +import org.apache.commons.lang3.StringUtils; + +import javax.inject.Inject; +import javax.inject.Singleton; +import java.net.URI; +import java.net.URISyntaxException; + +/** + * Utility for validating and sanitizing join addresses. + * + * @author AuroraLS3 + */ +@Singleton +public class JoinAddressValidator { + + private final PlanConfig config; + + @Inject + public JoinAddressValidator(PlanConfig config) { + /* Dagger injection constructor */ + this.config = config; + } + + @Untrusted + public String sanitize(@Untrusted String address) { + if (address == null) return ""; + if (!address.isEmpty()) { + // Remove port + if (address.contains(":")) { + address = address.substring(0, address.lastIndexOf(':')); + } + // Remove data added by Bungeecord/Velocity + if (address.contains("\u0000")) { + address = address.substring(0, address.indexOf('\u0000')); + } + // Remove data added by Forge Mod Loader + if (address.contains("fml")) { + address = address.substring(0, address.lastIndexOf("fml")); + } + if (config.isFalse(DataGatheringSettings.PRESERVE_JOIN_ADDRESS_CASE)) { + address = StringUtils.lowerCase(address); + } + } + return address; + } + + public boolean isValid(@Untrusted String address) { + if (address.isEmpty()) return false; + if (config.isTrue(DataGatheringSettings.PRESERVE_INVALID_JOIN_ADDRESS)) return true; + try { + URI uri = new URI(address); + String path = uri.getPath(); + return path != null && path.indexOf('.') != -1; + } catch (URISyntaxException uriSyntaxException) { + return false; + } + } + +} diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/PlatformPlayerData.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/PlatformPlayerData.java index d68027985..ed99d30a3 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/PlatformPlayerData.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/PlatformPlayerData.java @@ -16,6 +16,8 @@ */ package com.djrapitops.plan.gathering.domain; +import com.djrapitops.plan.utilities.dev.Untrusted; + import java.net.InetAddress; import java.util.Optional; import java.util.UUID; @@ -24,8 +26,10 @@ public interface PlatformPlayerData { UUID getUUID(); + @Untrusted String getName(); + @Untrusted default Optional getDisplayName() { return Optional.empty(); } @@ -38,6 +42,7 @@ public interface PlatformPlayerData { return Optional.empty(); } + @Untrusted default Optional getJoinAddress() { return Optional.empty(); } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/event/PlayerJoin.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/event/PlayerJoin.java index 65c53fe89..1d6b2ca16 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/event/PlayerJoin.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/gathering/domain/event/PlayerJoin.java @@ -64,6 +64,13 @@ public class PlayerJoin { return time; } + /** + * Get address used to join the server. + * + * @return Join address of the player. + * @deprecated {@link com.djrapitops.plan.gathering.JoinAddressValidator} should be used when looking at join address. + */ + @Deprecated(since = "2024-04-27") public String getJoinAddress() { return player.getJoinAddress().orElse(JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP); } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/gathering/events/PlayerJoinEventConsumer.java b/Plan/common/src/main/java/com/djrapitops/plan/gathering/events/PlayerJoinEventConsumer.java index 4ee09eb63..768b6037b 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/gathering/events/PlayerJoinEventConsumer.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/gathering/events/PlayerJoinEventConsumer.java @@ -22,6 +22,7 @@ import com.djrapitops.plan.delivery.domain.ServerName; import com.djrapitops.plan.delivery.export.Exporter; import com.djrapitops.plan.extension.CallEvents; import com.djrapitops.plan.extension.ExtensionSvc; +import com.djrapitops.plan.gathering.JoinAddressValidator; import com.djrapitops.plan.gathering.cache.NicknameCache; import com.djrapitops.plan.gathering.cache.SessionCache; import com.djrapitops.plan.gathering.domain.ActiveSession; @@ -38,7 +39,6 @@ import com.djrapitops.plan.storage.database.DBSystem; import com.djrapitops.plan.storage.database.sql.tables.JoinAddressTable; import com.djrapitops.plan.storage.database.transactions.Transaction; import com.djrapitops.plan.storage.database.transactions.events.*; -import org.apache.commons.lang3.StringUtils; import javax.inject.Inject; import javax.inject.Singleton; @@ -52,6 +52,7 @@ public class PlayerJoinEventConsumer { private final PlanConfig config; private final DBSystem dbSystem; + private final JoinAddressValidator joinAddressValidator; private final GeolocationCache geolocationCache; private final SessionCache sessionCache; private final NicknameCache nicknameCache; @@ -63,7 +64,7 @@ public class PlayerJoinEventConsumer { public PlayerJoinEventConsumer( Processing processing, PlanConfig config, - DBSystem dbSystem, + DBSystem dbSystem, JoinAddressValidator joinAddressValidator, GeolocationCache geolocationCache, SessionCache sessionCache, NicknameCache nicknameCache, @@ -73,6 +74,7 @@ public class PlayerJoinEventConsumer { this.processing = processing; this.config = config; this.dbSystem = dbSystem; + this.joinAddressValidator = joinAddressValidator; this.geolocationCache = geolocationCache; this.sessionCache = sessionCache; this.nicknameCache = nicknameCache; @@ -110,7 +112,8 @@ public class PlayerJoinEventConsumer { private void storeJoinAddress(PlayerJoin join) { join.getPlayer().getJoinAddress() - .map(joinAddress -> config.isTrue(DataGatheringSettings.PRESERVE_JOIN_ADDRESS_CASE) ? joinAddress : StringUtils.lowerCase(joinAddress)) + .map(joinAddressValidator::sanitize) + .filter(joinAddressValidator::isValid) .map(StoreJoinAddressTransaction::new) .ifPresent(dbSystem.getDatabase()::executeTransaction); } @@ -141,7 +144,10 @@ public class PlayerJoinEventConsumer { private CompletableFuture storeGamePlayer(PlayerJoin join) { long registerDate = getRegisterDate(join); - String joinAddress = join.getPlayer().getJoinAddress().orElse(JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP); + String joinAddress = join.getPlayer().getJoinAddress() + .map(joinAddressValidator::sanitize) + .filter(joinAddressValidator::isValid) + .orElse(JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP); Transaction transaction = new StoreServerPlayerTransaction( join.getPlayerUUID(), registerDate, join.getPlayer().getName(), join.getServerUUID(), joinAddress ); @@ -171,12 +177,16 @@ public class PlayerJoinEventConsumer { } private ActiveSession mapToActiveSession(PlayerJoin join) { + String joinAddress = join.getPlayer().getJoinAddress() + .map(joinAddressValidator::sanitize) + .filter(joinAddressValidator::isValid) + .orElse(JoinAddressTable.DEFAULT_VALUE_FOR_LOOKUP); ActiveSession session = new ActiveSession(join.getPlayerUUID(), join.getServerUUID(), join.getTime(), join.getPlayer().getCurrentWorld().orElse(null), join.getPlayer().getCurrentGameMode().orElse(null)); session.getExtraData().put(PlayerName.class, new PlayerName(join.getPlayer().getName())); session.getExtraData().put(ServerName.class, new ServerName(join.getServer().isProxy() ? join.getServer().getName() : "Proxy Server")); - session.getExtraData().put(JoinAddress.class, new JoinAddress(config.isTrue(DataGatheringSettings.PRESERVE_JOIN_ADDRESS_CASE) ? join.getJoinAddress() : StringUtils.lowerCase(join.getJoinAddress()))); + session.getExtraData().put(JoinAddress.class, new JoinAddress(joinAddress)); return session; } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/settings/config/paths/DataGatheringSettings.java b/Plan/common/src/main/java/com/djrapitops/plan/settings/config/paths/DataGatheringSettings.java index f39abbfd0..beaaa6f12 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/settings/config/paths/DataGatheringSettings.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/settings/config/paths/DataGatheringSettings.java @@ -35,6 +35,7 @@ public class DataGatheringSettings { public static final Setting LOG_UNKNOWN_COMMANDS = new BooleanSetting("Data_gathering.Commands.Log_unknown"); public static final Setting COMBINE_COMMAND_ALIASES = new BooleanSetting("Data_gathering.Commands.Log_aliases_as_main_command"); public static final Setting PRESERVE_JOIN_ADDRESS_CASE = new BooleanSetting("Data_gathering.Preserve_join_address_case"); + public static final Setting PRESERVE_INVALID_JOIN_ADDRESS = new BooleanSetting("Data_gathering.Preserve_invalid_join_addresses"); private DataGatheringSettings() { /* static variable class */ diff --git a/Plan/common/src/main/resources/assets/plan/bungeeconfig.yml b/Plan/common/src/main/resources/assets/plan/bungeeconfig.yml index fb8b2db8f..340fe348e 100644 --- a/Plan/common/src/main/resources/assets/plan/bungeeconfig.yml +++ b/Plan/common/src/main/resources/assets/plan/bungeeconfig.yml @@ -116,6 +116,7 @@ Data_gathering: Disk_space: true # Does not affect already gathered data Preserve_join_address_case: false + Preserve_invalid_join_addresses: false # ----------------------------------------------------- # Supported time units: MILLISECONDS, SECONDS, MINUTES, HOURS, DAYS # ----------------------------------------------------- diff --git a/Plan/common/src/main/resources/assets/plan/config.yml b/Plan/common/src/main/resources/assets/plan/config.yml index 39831916d..1aabbbbd6 100644 --- a/Plan/common/src/main/resources/assets/plan/config.yml +++ b/Plan/common/src/main/resources/assets/plan/config.yml @@ -120,6 +120,7 @@ Data_gathering: Log_aliases_as_main_command: true # Does not affect already gathered data Preserve_join_address_case: false + Preserve_invalid_join_addresses: false # ----------------------------------------------------- # Supported time units: MILLISECONDS, SECONDS, MINUTES, HOURS, DAYS # ----------------------------------------------------- diff --git a/Plan/common/src/test/java/com/djrapitops/plan/gathering/JoinAddressValidatorTest.java b/Plan/common/src/test/java/com/djrapitops/plan/gathering/JoinAddressValidatorTest.java new file mode 100644 index 000000000..88559559d --- /dev/null +++ b/Plan/common/src/test/java/com/djrapitops/plan/gathering/JoinAddressValidatorTest.java @@ -0,0 +1,88 @@ +/* + * This file is part of Player Analytics (Plan). + * + * Plan is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License v3 as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Plan is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Plan. If not, see . + */ +package com.djrapitops.plan.gathering; + +import com.djrapitops.plan.settings.config.PlanConfig; +import com.djrapitops.plan.settings.config.paths.DataGatheringSettings; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.when; + +/** + * @author AuroraLS3 + */ +@ExtendWith(MockitoExtension.class) +class JoinAddressValidatorTest { + + @Mock + PlanConfig config; + @InjectMocks + JoinAddressValidator joinAddressValidator; + + @DisplayName("Join address is valid") + @ParameterizedTest(name = "{0}") + @CsvSource({ + "play.domain.com", + "12.34.56.78", + "sub.play.domain.xz", + }) + void validJoinAddresses(String address) { + assertTrue(joinAddressValidator.isValid(address)); + } + + @DisplayName("Join address is invalid") + @ParameterizedTest(name = "{0}") + @CsvSource({ + "123", + "kioels8bfbc80hgjpz25uatt5bi1n0ueffoqxvrl+q7wgbgynl9jm2w38pihx1nw", // https://github.com/plan-player-analytics/Plan/issues/3545 + "play.domain.com:25565", + "play.domain.com:25565\u0000ouehfaounrfaeiurgea", + "play.domain.com\u0000h59783g9guheorig", + "PLAY.DOMAIN.COM:25565", + }) + void invalidJoinAddresses(String address) { + when(config.isTrue(DataGatheringSettings.PRESERVE_INVALID_JOIN_ADDRESS)).thenReturn(false); + assertFalse(joinAddressValidator.isValid(address)); + } + + @Test + @DisplayName("Empty join address is invalid") + void invalidEmptyJoinAddresses() { + assertFalse(joinAddressValidator.isValid("")); + } + + @DisplayName("Join address sanitization works") + @ParameterizedTest(name = "{0} -> {1}") + @CsvSource({ + "play.domain.com:25565, play.domain.com", + "play.domain.com:25565\u0000ouehfaounrfaeiurgea, play.domain.com", + "play.domain.com\u0000h59783g9guheorig, play.domain.com", + "play.domain.comfmlJEI=1.32.5, play.domain.com", + "PLAY.DOMAIN.COM:25565, PLAY.DOMAIN.COM", // Preserve case is on in the mocked config + }) + void sanitizationTest(String address, String expected) { + assertEquals(expected, joinAddressValidator.sanitize(address)); + } +} \ No newline at end of file diff --git a/Plan/fabric/src/main/java/net/playeranalytics/plan/gathering/listeners/fabric/PlayerOnlineListener.java b/Plan/fabric/src/main/java/net/playeranalytics/plan/gathering/listeners/fabric/PlayerOnlineListener.java index 11d9dd3a0..ab3b4e56b 100644 --- a/Plan/fabric/src/main/java/net/playeranalytics/plan/gathering/listeners/fabric/PlayerOnlineListener.java +++ b/Plan/fabric/src/main/java/net/playeranalytics/plan/gathering/listeners/fabric/PlayerOnlineListener.java @@ -16,6 +16,7 @@ */ package net.playeranalytics.plan.gathering.listeners.fabric; +import com.djrapitops.plan.gathering.JoinAddressValidator; import com.djrapitops.plan.gathering.cache.JoinAddressCache; import com.djrapitops.plan.gathering.domain.event.PlayerJoin; import com.djrapitops.plan.gathering.domain.event.PlayerLeave; @@ -51,6 +52,7 @@ public class PlayerOnlineListener implements FabricListener { private final PlayerJoinEventConsumer joinEventConsumer; private final PlayerLeaveEventConsumer leaveEventConsumer; private final JoinAddressCache joinAddressCache; + private final JoinAddressValidator joinAddressValidator; private final ServerInfo serverInfo; private final DBSystem dbSystem; @@ -66,7 +68,9 @@ public class PlayerOnlineListener implements FabricListener { public PlayerOnlineListener( PlayerJoinEventConsumer joinEventConsumer, PlayerLeaveEventConsumer leaveEventConsumer, - JoinAddressCache joinAddressCache, ServerInfo serverInfo, + JoinAddressCache joinAddressCache, + JoinAddressValidator joinAddressValidator, + ServerInfo serverInfo, DBSystem dbSystem, ErrorLogger errorLogger, MinecraftDedicatedServer server @@ -74,6 +78,7 @@ public class PlayerOnlineListener implements FabricListener { this.joinEventConsumer = joinEventConsumer; this.leaveEventConsumer = leaveEventConsumer; this.joinAddressCache = joinAddressCache; + this.joinAddressValidator = joinAddressValidator; this.serverInfo = serverInfo; this.dbSystem = dbSystem; this.errorLogger = errorLogger; @@ -127,10 +132,7 @@ public class PlayerOnlineListener implements FabricListener { private void onHandshake(HandshakeC2SPacket packet) { try { if (packet.intendedState() == ConnectionIntent.LOGIN) { - String address = packet.address(); - if (address != null && address.contains("\u0000")) { - address = address.substring(0, address.indexOf('\u0000')); - } + String address = joinAddressValidator.sanitize(packet.address()); joinAddress.set(address); } } catch (Exception e) { @@ -143,7 +145,10 @@ public class PlayerOnlineListener implements FabricListener { UUID playerUUID = profile.getId(); ServerUUID serverUUID = serverInfo.getServerUUID(); - joinAddressCache.put(playerUUID, joinAddress.get()); + String playerJoinAddress = joinAddress.get(); + if (joinAddressValidator.isValid(playerJoinAddress)) { + joinAddressCache.put(playerUUID, playerJoinAddress); + } dbSystem.getDatabase().executeTransaction(new BanStatusTransaction(playerUUID, serverUUID, banned)); } catch (Exception e) {