From 118b7a46eab84a5633e38570a32285ecf42d2e8b Mon Sep 17 00:00:00 2001 From: Ivan-Shaml <72102779+Ivan-Shaml@users.noreply.github.com> Date: Thu, 19 Dec 2024 17:28:40 +0200 Subject: [PATCH] fix(validation): on uploading map/mods add validation for version range --- .../com/faforever/api/error/ErrorCode.java | 2 ++ .../com/faforever/api/map/MapService.java | 12 ++++++++- .../com/faforever/api/mod/ModService.java | 22 ++++++++++++--- .../com/faforever/api/map/MapServiceTest.java | 17 ++++++++++++ .../com/faforever/api/mod/ModServiceTest.java | 10 +++++++ .../invalid_lua_map_version_scenario.lua | 27 +++++++++++++++++++ 6 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 src/test/resources/maps/scenario/invalid_lua_map_version_scenario.lua diff --git a/src/main/java/com/faforever/api/error/ErrorCode.java b/src/main/java/com/faforever/api/error/ErrorCode.java index 54ab607b2..68bdc9c06 100644 --- a/src/main/java/com/faforever/api/error/ErrorCode.java +++ b/src/main/java/com/faforever/api/error/ErrorCode.java @@ -118,6 +118,8 @@ public enum ErrorCode { MALFORMED_URL(208, "Malformed URL", "Provided url ''{0}'' is malformed."), NOT_ALLOWED_URL_HOST(209, "URL host not allowed", "Provided URL's host is not allowed. URL: ''{0}'', allowed hosts: ''{1}''."), STEAM_LOGIN_VALIDATION_FAILED(210, "Login via Steam failed", "Invalid OpenID redirect code"), + MAP_VERSION_INVALID_RANGE(211, "Invalid map version", "The map version must be a whole number in range {0, number} to {1, number}."), + MOD_VERSION_INVALID_RANGE(212, "Invalid mod version", "The mod version must be a whole number in range {0, number} to {1, number}."), ; diff --git a/src/main/java/com/faforever/api/map/MapService.java b/src/main/java/com/faforever/api/map/MapService.java index 22090ee22..27bb1eb2b 100644 --- a/src/main/java/com/faforever/api/map/MapService.java +++ b/src/main/java/com/faforever/api/map/MapService.java @@ -49,6 +49,7 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.OptionalInt; import java.util.regex.Pattern; import java.util.stream.Stream; @@ -75,6 +76,8 @@ public class MapService { private static final int MAP_NAME_MINUS_MAX_OCCURENCE = 3; private static final int MAP_NAME_MIN_LENGTH = 4; private static final int MAP_NAME_MAX_LENGTH = 50; + private static final int MAP_VERSION_MIN_VALUE = 1; + private static final int MAP_VERSION_MAX_VALUE = 9999; private static final String[] ADAPTIVE_REQUIRED_FILES = new String[]{ "_options.lua", @@ -311,8 +314,11 @@ private void validateScenarioLua(MapLuaAccessor mapLua, MapNameBuilder mapNameBu errors.add(new Error(ErrorCode.MAP_SIZE_MISSING)); } - if (mapLua.getMapVersion().isEmpty()) { + final OptionalInt mapVersion = mapLua.getMapVersion(); + if (mapVersion.isEmpty()) { errors.add(new Error(ErrorCode.MAP_VERSION_MISSING)); + } else if (!isMapVersionValidRange(mapVersion.getAsInt())) { + errors.add(new Error(ErrorCode.MAP_VERSION_INVALID_RANGE, MAP_VERSION_MIN_VALUE, MAP_VERSION_MAX_VALUE)); } if (mapLua.getNoRushRadius().isEmpty()) { @@ -325,6 +331,10 @@ private void validateScenarioLua(MapLuaAccessor mapLua, MapNameBuilder mapNameBu } } + private static boolean isMapVersionValidRange(int mapVersion) { + return mapVersion >= MAP_VERSION_MIN_VALUE && mapVersion <= MAP_VERSION_MAX_VALUE; + } + private Optional validateLuaPathVariable(MapLuaAccessor mapLua, String variableName, MapNameBuilder mapNameBuilder, String fileEnding) { String mapFileName = mapNameBuilder.buildFileName(fileEnding); String mapFolderNameWithoutVersion = mapNameBuilder.buildFolderNameWithoutVersion(); diff --git a/src/main/java/com/faforever/api/mod/ModService.java b/src/main/java/com/faforever/api/mod/ModService.java index c2c72427d..cb170879d 100644 --- a/src/main/java/com/faforever/api/mod/ModService.java +++ b/src/main/java/com/faforever/api/mod/ModService.java @@ -21,6 +21,7 @@ import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import org.apache.commons.io.FileUtils; +import org.apache.maven.artifact.versioning.ComparableVersion; import org.luaj.vm2.LuaValue; import org.springframework.cache.annotation.CacheEvict; import org.springframework.data.domain.Example; @@ -59,6 +60,9 @@ public class ModService { */ public static final String MOD_PATH_PREFIX = "mods/"; private static final Set ALLOWED_REPOSITORY_HOSTS = Set.of("github.com", "gitlab.com"); + private static final int MOD_VERSION_MIN_VALUE = 1; + private static final int MOD_VERSION_MAX_VALUE = 9999; + private final FafApiProperties properties; private final ModRepository modRepository; private final ModVersionRepository modVersionRepository; @@ -235,12 +239,20 @@ private void validateModInfo(com.faforever.commons.mod.Mod modInfo) { if (nullOrNil(modInfo.getUid())) { errors.add(new Error(ErrorCode.MOD_UID_MISSING)); } - if (modInfo.getVersion() == null || nullOrNil(modInfo.getVersion().toString())) { + + final ComparableVersion modVersion = modInfo.getVersion(); + if (modVersion == null || nullOrNil(modVersion.toString())) { errors.add(new Error(ErrorCode.MOD_VERSION_MISSING)); } - if (Ints.tryParse(modInfo.getVersion().toString()) == null) { - errors.add(new Error(ErrorCode.MOD_VERSION_NOT_A_NUMBER, modInfo.getVersion().toString())); + if (modVersion != null) { + final Integer versionInt = Ints.tryParse(modVersion.toString()); + if (versionInt == null) { + errors.add(new Error(ErrorCode.MOD_VERSION_NOT_A_NUMBER, modVersion.toString())); + } else if (!isModVersionValidRange(versionInt)){ + errors.add(new Error(ErrorCode.MOD_VERSION_INVALID_RANGE, MOD_VERSION_MIN_VALUE, MOD_VERSION_MAX_VALUE)); + } } + if (nullOrNil(modInfo.getDescription())) { errors.add(new Error(ErrorCode.MOD_DESCRIPTION_MISSING)); } @@ -253,6 +265,10 @@ private void validateModInfo(com.faforever.commons.mod.Mod modInfo) { } } + private static boolean isModVersionValidRange(int modVersion) { + return modVersion >= MOD_VERSION_MIN_VALUE && modVersion <= MOD_VERSION_MAX_VALUE; + } + @SneakyThrows private Optional extractThumbnail(Path modZipFile, short version, String displayName, String icon) { if (icon == null) { diff --git a/src/test/java/com/faforever/api/map/MapServiceTest.java b/src/test/java/com/faforever/api/map/MapServiceTest.java index 9ae49a609..adc4985c3 100644 --- a/src/test/java/com/faforever/api/map/MapServiceTest.java +++ b/src/test/java/com/faforever/api/map/MapServiceTest.java @@ -210,6 +210,23 @@ void testScenarioLuaWrongMapLine() { assertThat(mapLineError.getArgs()[0], is("map = '/maps/mirage/mirage.scmap'")); } + @Test + void testScenarioLuaMapVersionOutOfRange() { + ApiException result = assertThrows(ApiException.class, () -> + instance.validateScenarioLua(loadMapAsString("scenario/invalid_lua_map_version_scenario.lua"))); + + assertThat(result, hasErrorCodes( + ErrorCode.MAP_VERSION_INVALID_RANGE + )); + + assertThat(result.getErrors().length, is(1)); + Error mapLineError = Arrays.stream(result.getErrors()) + .filter(error -> error.getErrorCode() == ErrorCode.MAP_VERSION_INVALID_RANGE) + .findFirst().get(); + + assertThat(mapLineError.getArgs().length, is(2)); + } + @Test void authorBannedFromVault() { when(fafApiProperties.getMap()).thenReturn(new Map().setAllowedExtensions(Set.of("zip"))); diff --git a/src/test/java/com/faforever/api/mod/ModServiceTest.java b/src/test/java/com/faforever/api/mod/ModServiceTest.java index 3820031f8..067d91f9b 100644 --- a/src/test/java/com/faforever/api/mod/ModServiceTest.java +++ b/src/test/java/com/faforever/api/mod/ModServiceTest.java @@ -15,6 +15,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -222,6 +224,14 @@ public void testVersionNotANumber() throws Exception { assertThat(result, hasErrorCode(ErrorCode.MOD_VERSION_NOT_A_NUMBER)); } + @ParameterizedTest + @ValueSource(ints = {0, 10_000}) + public void testVersionOutOfRange(final int testVersion) throws Exception { + Path uploadFile = prepareModDynamic(luaContent().setVersion(Integer.toString(testVersion))); + ApiException result = assertThrows(ApiException.class, () -> instance.processUploadedMod(uploadFile, TEST_MOD_FILENAME, new Player(), null, null)); + assertThat(result, hasErrorCode(ErrorCode.MOD_VERSION_INVALID_RANGE)); + } + @Test public void testDescriptionMissing() throws Exception { Path uploadFile = prepareModDynamic(luaContent().setDesc(null)); diff --git a/src/test/resources/maps/scenario/invalid_lua_map_version_scenario.lua b/src/test/resources/maps/scenario/invalid_lua_map_version_scenario.lua new file mode 100644 index 000000000..a50658125 --- /dev/null +++ b/src/test/resources/maps/scenario/invalid_lua_map_version_scenario.lua @@ -0,0 +1,27 @@ +version = 3 -- Lua Version. Dont touch this +ScenarioInfo = { + name = "Mirage", + description = "Map By: Morax", + preview = '', + map_version = 10123, + type = 'skirmish', + starts = true, + size = {256, 256}, + map = '/maps/mirage/mirage.scmap', + save = '/maps/mirage/mirage_save.lua', + script = '/maps/mirage/mirage_script.lua', + norushradius = 40, + Configurations = { + ['standard'] = { + teams = { + { + name = 'FFA', + armies = {'ARMY_1', 'ARMY_2'} + }, + }, + customprops = { + ['ExtraArmies'] = STRING( 'ARMY_17 NEUTRAL_CIVILIAN' ), + }, + }, + }, +}