Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 + "'!");
Expand Down Expand Up @@ -120,4 +123,4 @@ public void run() {
}
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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";
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 + "}");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Allow the configured Bedrock prefix

When the documented default BedrockPlayerPrefix: '.' is used, a valid Bedrock vote such as .Player fails this Java-only pattern, so both new ingress checks reject it before the existing Bedrock-aware resolution runs; proxy cache or multi-proxy replays containing the resolved prefixed name are rejected as well. Recognize and validate the configured prefix plus base name, or delegate validation to the existing Bedrock-aware validation path.

Useful? React with 👍 / 👎.


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 "<null>";
}

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();
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,8 +70,6 @@ void testAddVoteParty() {
assertEquals(1, spyProxy.getVotePartyVotes());
}



@Test
void testAddCurrentVotePartyVotes() {
// Initial votePartyVotes should be 0
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -14,6 +16,11 @@
import com.bencodez.votingplugin.proxy.VotingPluginProxyConfig;

public class VotingPluginProxyTestImpl extends VotingPluginProxy {
private final List<String> warnings = new ArrayList<>();

public List<String> getWarnings() {
return warnings;
}

@Override
public void addNonVotedPlayer(String uuid, String playerName) {
Expand Down Expand Up @@ -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
Expand Down
Loading