diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/listeners/VotiferEvent.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/listeners/VotiferEvent.java index e12869967..c9aafc002 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/listeners/VotiferEvent.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/listeners/VotiferEvent.java @@ -6,8 +6,9 @@ import com.bencodez.simpleapi.array.ArrayUtils; import com.bencodez.votingplugin.VotingPluginMain; -import com.bencodez.votingplugin.events.PlayerVoteEvent; -import com.bencodez.votingplugin.proxy.BungeeMethod; +import com.bencodez.votingplugin.events.PlayerVoteEvent; +import com.bencodez.votingplugin.proxy.BungeeMethod; +import com.bencodez.votingplugin.util.MinecraftUsernameValidator; import com.vexsoftware.votifier.model.Vote; import com.vexsoftware.votifier.model.VotifierEvent; @@ -42,15 +43,17 @@ public void onVotiferEvent(VotifierEvent event) { } final String voteSite = str; final String IP = vote.getAddress(); - final String voteUsername = vote.getUsername().trim(); - if (IP.equals("VotingPlugin")) { - return; - } - - if (voteUsername.length() == 0) { - plugin.getLogger().warning("No name from vote on " + voteSite); - return; - } + final String voteUsername = vote.getUsername(); + if (IP.equals("VotingPlugin")) { + return; + } + + if (!MinecraftUsernameValidator.isValid(voteUsername)) { + plugin.getLogger().warning("Rejected vote with invalid Minecraft username '" + + MinecraftUsernameValidator.sanitizeForLog(voteUsername) + "' from service '" + + MinecraftUsernameValidator.sanitizeForLog(voteSite) + "'"); + return; + } plugin.getLogger() .info("Received a vote from service site '" + voteSite + "' by player '" + voteUsername + "'!"); @@ -120,4 +123,4 @@ public void run() { } }); } -} \ No newline at end of file +} diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/VotingPluginProxy.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/VotingPluginProxy.java index 4c035beac..ce79c3b04 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/VotingPluginProxy.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/VotingPluginProxy.java @@ -64,6 +64,7 @@ import com.bencodez.votingplugin.proxy.multiproxy.MultiProxyServerSocketConfigurationBungee; import com.bencodez.votingplugin.timequeue.VoteTimeQueue; import com.bencodez.votingplugin.topvoter.TopVoter; +import com.bencodez.votingplugin.util.MinecraftUsernameValidator; import com.bencodez.votingplugin.votelog.VoteLogMysqlTable; import com.bencodez.votingplugin.votelog.VoteLogMysqlTable.VoteLogStatus; import com.google.gson.JsonElement; @@ -1603,14 +1604,17 @@ public synchronized void vote(String player, String service, boolean realVote, b private synchronized void vote(String player, String service, boolean realVote, boolean timeQueue, long queueTime, VoteTotalsSnapshot text, String uuid, UUID existingVoteId) { try { + if (!MinecraftUsernameValidator.isValid(player)) { + warn("Rejected vote with invalid Minecraft username '" + + MinecraftUsernameValidator.sanitizeForLog(player) + "' from service '" + + MinecraftUsernameValidator.sanitizeForLog(service) + "'"); + return; + } + UUID voteId = existingVoteId; if (voteId == null) { voteId = UUID.randomUUID(); } - if (player == null || player.isEmpty()) { - log("No name from vote on " + service); - return; - } // Handle time change queue if (getConfig().getGlobalDataEnabled()) { diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/bungee/VoteEventBungee.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/bungee/VoteEventBungee.java index af28bf93c..79c41a464 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/bungee/VoteEventBungee.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/bungee/VoteEventBungee.java @@ -1,5 +1,6 @@ package com.bencodez.votingplugin.proxy.bungee; +import com.bencodez.votingplugin.util.MinecraftUsernameValidator; import com.vexsoftware.votifier.bungee.events.VotifierEvent; import com.vexsoftware.votifier.model.Vote; @@ -32,7 +33,8 @@ public void onVote(VotifierEvent event) { public void run() { Vote vote = event.getVote(); String serviceSite = vote.getServiceName(); - plugin.getLogger().info("Vote received " + vote.getUsername() + " from service site " + serviceSite); + plugin.getLogger().info("Vote received " + MinecraftUsernameValidator.sanitizeForLog(vote.getUsername()) + + " from service site " + MinecraftUsernameValidator.sanitizeForLog(serviceSite)); if (serviceSite.isEmpty()) { serviceSite = "Empty"; diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VoteEventVelocity.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VoteEventVelocity.java index ad03a27c9..b0199f28f 100644 --- a/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VoteEventVelocity.java +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VoteEventVelocity.java @@ -1,5 +1,6 @@ package com.bencodez.votingplugin.proxy.velocity; +import com.bencodez.votingplugin.util.MinecraftUsernameValidator; import com.velocitypowered.api.event.Subscribe; import com.vexsoftware.votifier.velocity.event.VotifierEvent; @@ -30,7 +31,8 @@ public void onVotifierEvent(VotifierEvent event) { @Override public void run() { String serviceSite = serviceSiteVote; - plugin.getLogger().info("Vote received " + name + " from service site " + serviceSite); + plugin.getLogger().info("Vote received " + MinecraftUsernameValidator.sanitizeForLog(name) + + " from service site " + MinecraftUsernameValidator.sanitizeForLog(serviceSite)); if (serviceSite.isEmpty()) { serviceSite = "Empty"; } diff --git a/VotingPlugin/src/main/java/com/bencodez/votingplugin/util/MinecraftUsernameValidator.java b/VotingPlugin/src/main/java/com/bencodez/votingplugin/util/MinecraftUsernameValidator.java new file mode 100644 index 000000000..a371e98f4 --- /dev/null +++ b/VotingPlugin/src/main/java/com/bencodez/votingplugin/util/MinecraftUsernameValidator.java @@ -0,0 +1,53 @@ +package com.bencodez.votingplugin.util; + +import java.util.regex.Pattern; + +/** + * Validates Java Edition player names received from external vote sources. + */ +public final class MinecraftUsernameValidator { + private static final int MAX_LENGTH = 16; + private static final int MAX_LOG_LENGTH = 32; + private static final Pattern VALID_USERNAME = Pattern.compile("[A-Za-z0-9_]{1," + MAX_LENGTH + "}"); + + private MinecraftUsernameValidator() { + } + + /** + * Tests whether a value uses the Minecraft Java username syntax. + * + * @param username username supplied by an external source + * @return {@code true} for a 1-16 character ASCII letter, digit, or underscore + */ + public static boolean isValid(String username) { + return username != null && VALID_USERNAME.matcher(username).matches(); + } + + /** + * Produces a bounded, single-line representation safe to include in logs. + * + * @param value untrusted external value + * @return sanitized value + */ + public static String sanitizeForLog(String value) { + if (value == null) { + return ""; + } + + StringBuilder sanitized = new StringBuilder(Math.min(value.length(), MAX_LOG_LENGTH)); + int length = Math.min(value.length(), MAX_LOG_LENGTH); + for (int i = 0; i < length; i++) { + char character = value.charAt(i); + if ((character >= 'A' && character <= 'Z') || (character >= 'a' && character <= 'z') + || (character >= '0' && character <= '9') || character == '_') { + sanitized.append(character); + } else { + sanitized.append('?'); + } + } + if (value.length() > MAX_LOG_LENGTH) { + sanitized.append("..."); + } + return sanitized.toString(); + } +} diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/MinecraftUsernameValidatorTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/MinecraftUsernameValidatorTest.java new file mode 100644 index 000000000..b6bd96236 --- /dev/null +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/MinecraftUsernameValidatorTest.java @@ -0,0 +1,27 @@ +package com.bencodez.votingplugin.tests; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +import com.bencodez.votingplugin.util.MinecraftUsernameValidator; + +class MinecraftUsernameValidatorTest { + + @Test + void acceptsValidMinecraftUsernames() { + for (String username : new String[] { "MchtName15Chars", "MchtNameOver16xx", "Player_123" }) { + assertTrue(MinecraftUsernameValidator.isValid(username), username); + } + } + + @Test + void rejectsInvalidMinecraftUsernames() { + for (String username : new String[] { "MchtNameOver16xxx", "../MchtTraversal", "Mcht/Slash", "Mcht\\Slash", + "Mcht Space", "Mcht\tTab", "Mcht\nLine", "Mcht\u0000Control", "Mcht- punctuation", + "Mcht\u00E9Unicode", "\uD800" }) { + assertFalse(MinecraftUsernameValidator.isValid(username), username); + } + } +} diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTest.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTest.java index 5bc887888..74c5ec9d1 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTest.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTest.java @@ -2,7 +2,10 @@ package com.bencodez.votingplugin.tests; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.junit.jupiter.api.BeforeEach; @@ -67,8 +70,6 @@ void testAddVoteParty() { assertEquals(1, spyProxy.getVotePartyVotes()); } - - @Test void testAddCurrentVotePartyVotes() { // Initial votePartyVotes should be 0 @@ -82,4 +83,23 @@ void testAddCurrentVotePartyVotes() { votingPluginProxy.addCurrentVotePartyVotes(2); assertEquals(5, votingPluginProxy.getVotePartyVotes()); } + + @Test + void invalidVoteStopsBeforeAnyPersistentRewardCacheOrForwardingState() { + for (String username : new String[] { "MchtNameOver16xxx", "../MchtTraversal", "Mcht/Slash", "Mcht\\Slash", + "Mcht Space", "Mcht\tTab", "Mcht\u00E9Unicode" }) { + VotingPluginProxyTestImpl spyProxy = Mockito.spy(votingPluginProxy); + + spyProxy.vote(username, "MCHT", true, true, 0, null, null); + + verify(spyProxy, never()).getUUID(Mockito.anyString()); + verify(spyProxy, never()).addVoteParty(); + verify(spyProxy, never()).getVoteCacheHandler(); + verify(spyProxy, never()).getGlobalMessageProxyHandler(); + verify(proxyMySQL, never()).containsKeyQuery(Mockito.anyString()); + verify(multiProxyHandler, never()).sendMultiProxyEnvelope(Mockito.any()); + assertEquals(0, spyProxy.getVotePartyVotes(), username); + assertTrue(spyProxy.getWarnings().stream().anyMatch(warning -> warning.contains("Rejected vote")), username); + } + } } diff --git a/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTestImpl.java b/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTestImpl.java index 3f984db79..5ab8a546b 100644 --- a/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTestImpl.java +++ b/VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTestImpl.java @@ -1,8 +1,10 @@ package com.bencodez.votingplugin.tests; import java.io.File; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.UUID; import java.util.concurrent.ScheduledExecutorService; @@ -14,6 +16,11 @@ import com.bencodez.votingplugin.proxy.VotingPluginProxyConfig; public class VotingPluginProxyTestImpl extends VotingPluginProxy { + private final List warnings = new ArrayList<>(); + + public List getWarnings() { + return warnings; + } @Override public void addNonVotedPlayer(String uuid, String playerName) { @@ -144,8 +151,7 @@ public void logSevere(String message) { @Override public void warn(String message) { - // For testing, simply print the warning message - System.err.println("WARNING: " + message); + warnings.add(message); } @Override