#1104 Whitelist sensitive commands also when used with "authme:" prefix

This commit is contained in:
ljacqu 2017-02-21 22:51:45 +01:00
parent ee51bb3971
commit 4edb4e68c2
4 changed files with 40 additions and 18 deletions

View File

@ -3,6 +3,10 @@ package fr.xephi.authme.output;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.StringUtils;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
/** /**
* Service class for the log filters. * Service class for the log filters.
*/ */
@ -11,11 +15,10 @@ final class LogFilterHelper {
private static final String ISSUED_COMMAND_TEXT = "issued server command:"; private static final String ISSUED_COMMAND_TEXT = "issued server command:";
@VisibleForTesting @VisibleForTesting
static final String[] COMMANDS_TO_SKIP = { static final List<String> COMMANDS_TO_SKIP = withAndWithoutAuthMePrefix(
"/login ", "/l ", "/log ", "/register ", "/reg ", "/unregister ", "/unreg ", "/login ", "/l ", "/log ", "/register ", "/reg ", "/unregister ", "/unreg ",
"/changepassword ", "/cp ", "/changepass ", "/authme register ", "/authme reg ", "/authme r ", "/changepassword ", "/cp ", "/changepass ", "/authme register ", "/authme reg ", "/authme r ",
"/authme changepassword ", "/authme password ", "/authme changepass ", "/authme cp " "/authme changepassword ", "/authme password ", "/authme changepass ", "/authme cp ");
};
private LogFilterHelper() { private LogFilterHelper() {
// Util class // Util class
@ -35,4 +38,13 @@ final class LogFilterHelper {
String lowerMessage = message.toLowerCase(); String lowerMessage = message.toLowerCase();
return lowerMessage.contains(ISSUED_COMMAND_TEXT) && StringUtils.containsAny(lowerMessage, COMMANDS_TO_SKIP); return lowerMessage.contains(ISSUED_COMMAND_TEXT) && StringUtils.containsAny(lowerMessage, COMMANDS_TO_SKIP);
} }
private static List<String> withAndWithoutAuthMePrefix(String... commands) {
List<String> commandList = new ArrayList<>(commands.length * 2);
for (String command : commands) {
commandList.add(command);
commandList.add(command.substring(0, 1) + "authme:" + command.substring(1));
}
return Collections.unmodifiableList(commandList);
}
} }

View File

@ -42,7 +42,7 @@ public final class StringUtils {
* *
* @return True if the string contains at least one of the items * @return True if the string contains at least one of the items
*/ */
public static boolean containsAny(String str, String... pieces) { public static boolean containsAny(String str, Iterable<String> pieces) {
if (str == null) { if (str == null) {
return false; return false;
} }

View File

@ -1,6 +1,6 @@
package fr.xephi.authme.output; package fr.xephi.authme.output;
import com.google.common.base.Preconditions; import com.google.common.collect.Lists;
import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandDescription;
import fr.xephi.authme.command.CommandInitializer; import fr.xephi.authme.command.CommandInitializer;
import org.junit.Test; import org.junit.Test;
@ -38,7 +38,7 @@ public class LogFilterHelperTest {
.toArray(String[]::new); .toArray(String[]::new);
// when / then // when / then
assertThat(Arrays.asList(LogFilterHelper.COMMANDS_TO_SKIP), containsInAnyOrder(expectedEntries)); assertThat(LogFilterHelper.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries));
} }
@ -59,21 +59,30 @@ public class LogFilterHelperTest {
/** /**
* Returns all "command syntaxes" from which the given command can be reached. * Returns all "command syntaxes" from which the given command can be reached.
* For example, the result might be a List containing "/authme changepassword", "/authme changepass" * For example, the result might be a List containing "/authme changepassword", "/authme changepass",
* and "/authme cp". * "/authme cp", "/authme:authme changepassword" etc.
* *
* @param command the command to build syntaxes for * @param command the command to build syntaxes for
* @return command syntaxes * @return command syntaxes
*/ */
private static List<String> buildCommandSyntaxes(CommandDescription command) { private static List<String> buildCommandSyntaxes(CommandDescription command) {
// assumes that parent can only have one label -> if this fails in the future, we need to revise this method List<String> prefixes = getCommandPrefixes(command);
Preconditions.checkArgument(command.getParent() == null || command.getParent().getLabels().size() == 1);
String prefix = command.getParent() == null return command.getLabels()
? "/" .stream()
: "/" + command.getParent().getLabels().get(0) + " "; .map(label -> Lists.transform(prefixes, p -> p + label))
return command.getLabels().stream() .flatMap(List::stream)
.map(label -> prefix + label) .collect(Collectors.toList());
}
private static List<String> getCommandPrefixes(CommandDescription command) {
if (command.getParent() == null) {
return Arrays.asList("/", "/authme:");
}
return command.getParent().getLabels()
.stream()
.map(label -> new String[]{"/" + label + " ", "/authme:" + label + " "})
.flatMap(Arrays::stream)
.collect(Collectors.toList()); .collect(Collectors.toList());
} }
} }

View File

@ -5,6 +5,7 @@ import org.junit.Test;
import java.net.MalformedURLException; import java.net.MalformedURLException;
import static java.util.Arrays.asList;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThan;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
@ -23,7 +24,7 @@ public class StringUtilsTest {
String piece = "test"; String piece = "test";
// when // when
boolean result = StringUtils.containsAny(text, "some", "words", "that", "do not", "exist", piece); boolean result = StringUtils.containsAny(text, asList("some", "words", "that", "do not", "exist", piece));
// then // then
assertThat(result, equalTo(true)); assertThat(result, equalTo(true));
@ -35,7 +36,7 @@ public class StringUtilsTest {
String text = "This is a test string"; String text = "This is a test string";
// when // when
boolean result = StringUtils.containsAny(text, "some", "other", "words", null); boolean result = StringUtils.containsAny(text, asList("some", "other", "words", null));
// then // then
assertThat(result, equalTo(false)); assertThat(result, equalTo(false));
@ -44,7 +45,7 @@ public class StringUtilsTest {
@Test @Test
public void shouldReturnFalseForNullString() { public void shouldReturnFalseForNullString() {
// given/when // given/when
boolean result = StringUtils.containsAny(null, "some", "words", "to", "check"); boolean result = StringUtils.containsAny(null, asList("some", "words", "to", "check"));
// then // then
assertThat(result, equalTo(false)); assertThat(result, equalTo(false));