Upstream/Plan
1
0
Fork 0
mirror of https://github.com/plan-player-analytics/Plan.git synced 2024-06-22 12:45:04 +02:00
Code Issues Projects Releases Wiki Activity

Test against and identify path traversal vulnerability in other methods

Browse Source
This commit is contained in:
Aurora Lahtela 2023-01-15 09:30:30 +02:00
parent b0a1bc1fb1
commit f20a04809c
8 changed files with 102 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
Plan/common/src/main/java/com/djrapitops/plan/delivery/web/ResourceSvc.java
View File

@ -23,6 +23,7 @@ import com.djrapitops.plan.settings.locale.Locale;
import com.djrapitops.plan.settings.locale.lang.PluginLang;
import com.djrapitops.plan.storage.file.PlanFiles;
import com.djrapitops.plan.storage.file.Resource;
import com.djrapitops.plan.utilities.dev.Untrusted;
import com.djrapitops.plan.utilities.logging.ErrorContext;
import com.djrapitops.plan.utilities.logging.ErrorLogger;
import net.playeranalytics.plugin.server.PluginLogger;
@ -76,12 +77,12 @@ public class ResourceSvc implements ResourceService {
}
@Override
public WebResource getResource(String pluginName, String fileName, Supplier<WebResource> source) {
public WebResource getResource(String pluginName, @Untrusted String fileName, Supplier<WebResource> source) {
checkParams(pluginName, fileName, source);
return applySnippets(pluginName, fileName, getTheResource(pluginName, fileName, source));
}
public void checkParams(String pluginName, String fileName, Supplier<WebResource> source) {
public void checkParams(String pluginName, @Untrusted String fileName, Supplier<WebResource> source) {
if (pluginName == null || pluginName.isEmpty()) {
throw new IllegalArgumentException("'pluginName' can't be '" + pluginName + "'!");
}
@ -93,7 +94,7 @@ public class ResourceSvc implements ResourceService {
}
}
private WebResource applySnippets(String pluginName, String fileName, WebResource resource) {
private WebResource applySnippets(String pluginName, @Untrusted String fileName, WebResource resource) {
Map<Position, StringBuilder> byPosition = calculateSnippets(fileName);
if (byPosition.isEmpty()) return resource;
@ -129,7 +130,7 @@ public class ResourceSvc implements ResourceService {
return html;
}
private Map<Position, StringBuilder> calculateSnippets(String fileName) {
private Map<Position, StringBuilder> calculateSnippets(@Untrusted String fileName) {
Map<Position, StringBuilder> byPosition = new EnumMap<>(Position.class);
for (Snippet snippet : snippets) {
if (snippet.matches(fileName)) {
@ -139,7 +140,7 @@ public class ResourceSvc implements ResourceService {
return byPosition;
}
public WebResource getTheResource(String pluginName, String fileName, Supplier<WebResource> source) {
public WebResource getTheResource(String pluginName, @Untrusted String fileName, Supplier<WebResource> source) {
try {
if (resourceSettings.shouldBeCustomized(pluginName, fileName)) {
return getOrWriteCustomized(fileName, source);
@ -153,7 +154,7 @@ public class ResourceSvc implements ResourceService {
return source.get();
}
public WebResource getOrWriteCustomized(String fileName, Supplier<WebResource> source) throws IOException {
public WebResource getOrWriteCustomized(@Untrusted String fileName, Supplier<WebResource> source) throws IOException {
Optional<Resource> customizedResource = files.getCustomizableResource(fileName);
if (customizedResource.isPresent()) {
return readCustomized(customizedResource.get());
@ -170,11 +171,16 @@ public class ResourceSvc implements ResourceService {
}
}
public WebResource writeCustomized(String fileName, Supplier<WebResource> source) throws IOException {
public WebResource writeCustomized(@Untrusted String fileName, Supplier<WebResource> source) throws IOException {
WebResource original = source.get();
byte[] bytes = original.asBytes();
OpenOption[] overwrite = {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE};
Path to = resourceSettings.getCustomizationDirectory().resolve(fileName);
@Untrusted Path to = resourceSettings.getCustomizationDirectory().resolve(fileName);
if (!to.startsWith(resourceSettings.getCustomizationDirectory())) {
throw new IllegalArgumentException(
"Absolute path was given for writing a customized file, " +
"writing outside customization directory is prevented for security reasons.");
}
Path dir = to.getParent();
if (!Files.isSymbolicLink(dir)) Files.createDirectories(dir);
Files.write(to, bytes, overwrite);

4
Plan/common/src/main/java/com/djrapitops/plan/delivery/webserver/ResponseFactory.java
View File

@ -84,7 +84,7 @@ public class ResponseFactory {
this.addresses = addresses;
}
public WebResource getResource(String resourceName) {
public WebResource getResource(@Untrusted String resourceName) {
return ResourceService.getInstance().getResource("Plan", resourceName,
() -> files.getResourceFromJar("web/" + resourceName).asWebResource());
}
@ -205,7 +205,7 @@ public class ResponseFactory {
return StringUtils.replace(resource, "PLAN_BASE_ADDRESS", address);
}
public Response cssResponse(String fileName) {
public Response cssResponse(@Untrusted String fileName) {
try {
String content = UnaryChain.of(getResource(fileName).asString())
.chain(theme::replaceThemeColors)

5
Plan/common/src/main/java/com/djrapitops/plan/settings/config/ResourceSettings.java
View File

@ -19,6 +19,7 @@ package com.djrapitops.plan.settings.config;
import com.djrapitops.plan.settings.config.paths.CustomizedFileSettings;
import com.djrapitops.plan.settings.config.paths.PluginSettings;
import com.djrapitops.plan.storage.file.PlanFiles;
import com.djrapitops.plan.utilities.dev.Untrusted;
import org.apache.commons.lang3.StringUtils;
import java.io.IOException;
@ -39,7 +40,7 @@ public class ResourceSettings {
this.config = config;
}
public boolean shouldBeCustomized(String plugin, String fileName) {
public boolean shouldBeCustomized(String plugin, @Untrusted String fileName) {
if (config.isTrue(CustomizedFileSettings.WEB_DEV_MODE) && config.isFalse(PluginSettings.FRONTEND_BETA)) {
return true;
}
@ -48,6 +49,8 @@ public class ResourceSettings {
fileCustomization.setComment(Collections.singletonList("The files are placed in /Plan/web/ if the setting is 'true' when accessed."));
ConfigNode pluginCustomization = fileCustomization.getNode(plugin).orElseGet(() -> fileCustomization.addNode(plugin));
// No longer untrusted in configuration context, but may contain untrusted data in other context.
String fileNameNonPath = StringUtils.replaceChars(fileName, '.', ',');
if (pluginCustomization.contains(fileNameNonPath)) {

6
Plan/common/src/main/java/com/djrapitops/plan/storage/file/FileResource.java
View File

@ -16,6 +16,8 @@
*/
package com.djrapitops.plan.storage.file;
import com.djrapitops.plan.utilities.dev.Untrusted;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
@ -35,10 +37,11 @@ import java.util.stream.Stream;
*/
public class FileResource implements Resource {
@Untrusted
private final String resourceName;
private final File file;
public FileResource(String resourceName, File file) {
public FileResource(@Untrusted String resourceName, File file) {
this.resourceName = resourceName;
this.file = file;
}
@ -54,6 +57,7 @@ public class FileResource implements Resource {
}
@Override
@Untrusted
public String getResourceName() {
return resourceName;
}

14
Plan/common/src/main/java/com/djrapitops/plan/storage/file/PlanFiles.java
View File

@ -119,7 +119,7 @@ public class PlanFiles implements SubSystem {
* @param resourceName Path to the file inside jar/assets/plan/ folder.
* @return a {@link Resource} for accessing the resource.
*/
public Resource getResourceFromJar(String resourceName) {
public Resource getResourceFromJar(@Untrusted String resourceName) {
return new JarResource("assets/plan/" + resourceName, getResourceStream);
}
@ -134,11 +134,11 @@ public class PlanFiles implements SubSystem {
}
// TODO Customized file logic should be moved to another class so the circular dependency on config can be removed.
public Optional<Resource> getCustomizableResource(String resourceName) {
public Optional<Resource> getCustomizableResource(@Untrusted String resourceName) {
return Optional.ofNullable(findCustomized(resourceName));
}
private Resource findCustomized(String resourceName) {
private Resource findCustomized(@Untrusted String resourceName) {
if (config.get().isTrue(CustomizedFileSettings.WEB_DEV_MODE)) {
// Bypass cache in web developer mode.
return getFileResource(resourceName);
@ -147,19 +147,21 @@ public class PlanFiles implements SubSystem {
}
}
private FileResource getFileResource(String resourceName) {
private FileResource getFileResource(@Untrusted String resourceName) {
return attemptToFind(resourceName)
.map(found -> new FileResource(resourceName, found))
.orElse(null);
}
public Optional<File> attemptToFind(String resourceName) {
public Optional<File> attemptToFind(@Untrusted String resourceName) {
Path dir = config.get().getResourceSettings().getCustomizationDirectory();
if (dir.toFile().exists() && dir.toFile().isDirectory()) {
Path asPath = dir.resolve(resourceName);
// Path may be absolute due to resolving untrusted path
@Untrusted Path asPath = dir.resolve(resourceName);
if (!asPath.startsWith(dir)) {
return Optional.empty();
}
// Now it should be trustworthy
File found = asPath.toFile();
if (found.exists()) {
return Optional.of(found);

2
Plan/common/src/main/java/com/djrapitops/plan/storage/file/Resource.java
View File

@ -17,6 +17,7 @@
package com.djrapitops.plan.storage.file;
import com.djrapitops.plan.delivery.web.resource.WebResource;
import com.djrapitops.plan.utilities.dev.Untrusted;
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
import org.apache.commons.lang3.StringUtils;
@ -38,6 +39,7 @@ public interface Resource {
*
* @return Relative file path given to {@link PlanFiles}.
*/
@Untrusted
String getResourceName();
byte[] asBytes() throws IOException;

3
Plan/common/src/main/java/com/djrapitops/plan/storage/file/ResourceCache.java
View File

@ -16,6 +16,7 @@
*/
package com.djrapitops.plan.storage.file;
import com.djrapitops.plan.utilities.dev.Untrusted;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
@ -40,7 +41,7 @@ public class ResourceCache {
// Static class
}
public static Resource getOrCache(String resourceName, Supplier<Resource> resourceSupplier) {
public static Resource getOrCache(@Untrusted String resourceName, Supplier<Resource> resourceSupplier) {
String found = cache.getIfPresent(resourceName);
if (found == null) {
Resource created = resourceSupplier.get();

65
Plan/common/src/test/java/com/djrapitops/plan/storage/file/PlanFilesTest.java Normal file
View File

@ -0,0 +1,65 @@
/*
* 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 <https://www.gnu.org/licenses/>.
*/
package com.djrapitops.plan.storage.file;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.settings.config.paths.CustomizedFileSettings;
import extension.FullSystemExtension;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* @author AuroraLS3
*/
@ExtendWith(FullSystemExtension.class)
class PlanFilesTest {
@Test
@DisplayName("getFileFromPluginFolder has no Path Traversal vulnerability")
void getFileFromPluginFolderDoesNotAllowAbsolutePathTraversal(@TempDir Path tempDir, PlanFiles files) throws IOException {
Path testFile = tempDir.resolve("file.db");
Files.createDirectories(tempDir.getParent());
Files.createFile(testFile);
File file = files.getFileFromPluginFolder(testFile.toFile().getAbsolutePath());
assertNotEquals(testFile.toFile().getAbsolutePath(), file.getAbsolutePath());
}
@Test
@DisplayName("getCustomizableResource has no Path Traversal vulnerability")
void getCustomizableResourceDoesNotAllowAbsolutePathTraversal(@TempDir Path tempDir, PlanConfig config, PlanFiles files) throws IOException {
config.set(CustomizedFileSettings.PATH, tempDir.resolve("customized").toFile().getAbsolutePath());
Path testFile = tempDir.resolve("file.db");
Files.createDirectories(tempDir.getParent());
Files.createFile(testFile);
Optional<Resource> resource = files.getCustomizableResource(testFile.toFile().getAbsolutePath());
assertTrue(resource.isEmpty());
}
}