From 8d8dae171e1242bab9d28e055b7ca26316312321 Mon Sep 17 00:00:00 2001 From: Lunar Starstrum Date: Sun, 1 Mar 2026 15:42:33 -0600 Subject: [PATCH 1/3] Switch to Netty to fix vulnerabilities --- common/pom.xml | 16 +- .../java/com/loohp/imageframe/Commands.java | 136 +++-- .../java/com/loohp/imageframe/ImageFrame.java | 56 +- .../imageframe/upload/ImageUploadManager.java | 479 ++++++++++-------- .../imageframe/upload/PendingUpload.java | 43 +- common/src/main/resources/config.yml | 17 +- 6 files changed, 438 insertions(+), 309 deletions(-) diff --git a/common/pom.xml b/common/pom.xml index 453ed25..8cef409 100644 --- a/common/pom.xml +++ b/common/pom.xml @@ -105,6 +105,10 @@ org.apache.commons com.loohp.imageframe.libs.org.apache.commons + + io.netty + com.loohp.imageframe.libs.io.netty + @@ -220,12 +224,12 @@ - spigotmc-repo - https://hub.spigotmc.org/nexus/content/repositories/snapshots/ + spigot-repo + https://hub.spigotmc.org/nexus/content/groups/public/ paper-repo - https://papermc.io/repo/repository/maven-public/ + https://repo.papermc.io/repository/maven-public/ sonatype @@ -263,6 +267,12 @@ + + io.netty + netty-all + 4.2.10.Final + compile + com.zaxxer HikariCP diff --git a/common/src/main/java/com/loohp/imageframe/Commands.java b/common/src/main/java/com/loohp/imageframe/Commands.java index 3f526db..05d5c2e 100644 --- a/common/src/main/java/com/loohp/imageframe/Commands.java +++ b/common/src/main/java/com/loohp/imageframe/Commands.java @@ -20,6 +20,100 @@ package com.loohp.imageframe; +import static com.loohp.imageframe.language.TranslationKey.ACCESS_TYPE; +import static com.loohp.imageframe.language.TranslationKey.ACCESS_UPDATED; +import static com.loohp.imageframe.language.TranslationKey.DUPLICATE_MAP_NAME; +import static com.loohp.imageframe.language.TranslationKey.GIVEN_INVISIBLE_FRAME; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_ALREADY_QUEUED; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_CREATED; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_DELETED; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_PLAYBACK_JUMP_TO; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_PLAYER_PURGE; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_PROCESSING; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_REFRESHED; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_RENAMED; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_MAP_TOGGLE_PAUSED; +import static com.loohp.imageframe.language.TranslationKey.IMAGE_OVER_MAX_FILE_SIZE; +import static com.loohp.imageframe.language.TranslationKey.INVALID_OVERLAY_MAP; +import static com.loohp.imageframe.language.TranslationKey.INVALID_USAGE; +import static com.loohp.imageframe.language.TranslationKey.ITEM_FRAME_OCCUPIED; +import static com.loohp.imageframe.language.TranslationKey.MAP_LOOKUP; +import static com.loohp.imageframe.language.TranslationKey.MARKERS_ADD_BEGIN; +import static com.loohp.imageframe.language.TranslationKey.MARKERS_CANCEL; +import static com.loohp.imageframe.language.TranslationKey.MARKERS_CLEAR; +import static com.loohp.imageframe.language.TranslationKey.MARKERS_DUPLICATE_NAME; +import static com.loohp.imageframe.language.TranslationKey.MARKERS_NOT_A_MARKER; +import static com.loohp.imageframe.language.TranslationKey.MARKERS_NOT_RENDER_WARNING; +import static com.loohp.imageframe.language.TranslationKey.MARKERS_REMOVE; +import static com.loohp.imageframe.language.TranslationKey.NOT_AN_IMAGE_MAP; +import static com.loohp.imageframe.language.TranslationKey.NOT_ENOUGH_MAPS; +import static com.loohp.imageframe.language.TranslationKey.NO_CONSOLE; +import static com.loohp.imageframe.language.TranslationKey.NO_PERMISSION; +import static com.loohp.imageframe.language.TranslationKey.OVERSIZE; +import static com.loohp.imageframe.language.TranslationKey.PLAYER_CREATION_LIMIT_REACHED; +import static com.loohp.imageframe.language.TranslationKey.PLAYER_NOT_FOUND; +import static com.loohp.imageframe.language.TranslationKey.PREFERENCES_TYPE; +import static com.loohp.imageframe.language.TranslationKey.PREFERENCES_UPDATE; +import static com.loohp.imageframe.language.TranslationKey.PREFERENCES_VALUE; +import static com.loohp.imageframe.language.TranslationKey.RELOADED; +import static com.loohp.imageframe.language.TranslationKey.RESYNC; +import static com.loohp.imageframe.language.TranslationKey.SELECTION_BEGIN; +import static com.loohp.imageframe.language.TranslationKey.SELECTION_CLEAR; +import static com.loohp.imageframe.language.TranslationKey.SELECTION_INCORRECT_SIZE; +import static com.loohp.imageframe.language.TranslationKey.SELECTION_INVALID; +import static com.loohp.imageframe.language.TranslationKey.SELECTION_NO_SELECTION; +import static com.loohp.imageframe.language.TranslationKey.SET_CREATOR; +import static com.loohp.imageframe.language.TranslationKey.STORAGE_MIGRATION; +import static com.loohp.imageframe.language.TranslationKey.UNABLE_TO_CHANGE_IMAGE_TYPE; +import static com.loohp.imageframe.language.TranslationKey.UNABLE_TO_LOAD_MAP; +import static com.loohp.imageframe.language.TranslationKey.UNKNOWN_ERROR; +import static com.loohp.imageframe.language.TranslationKey.UPLOAD_EXPIRED; +import static com.loohp.imageframe.language.TranslationKey.UPLOAD_LINK; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_1; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_2; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_3; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_4; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_5; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_6; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_7; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_8; +import static com.loohp.imageframe.language.TranslationKey.URL_IMAGE_MAP_INFO_9; +import static com.loohp.imageframe.language.TranslationKey.URL_RESTRICTED; +import static com.loohp.imageframe.utils.CommandSenderUtils.sendMessage; +import static com.loohp.imageframe.utils.ComponentUtils.translatable; +import static net.kyori.adventure.text.Component.text; + +import java.io.IOException; +import java.net.URLConnection; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.Date; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.PrimitiveIterator; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Stream; + +import org.bukkit.Bukkit; +import org.bukkit.Material; +import org.bukkit.OfflinePlayer; +import org.bukkit.World; +import org.bukkit.command.Command; +import org.bukkit.command.CommandExecutor; +import org.bukkit.command.CommandSender; +import org.bukkit.command.TabCompleter; +import org.bukkit.entity.Player; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.ItemStack; +import org.bukkit.map.MapCursor; +import org.bukkit.map.MapView; + import com.loohp.imageframe.api.events.ImageMapUpdatedEvent; import com.loohp.imageframe.migration.ExternalPluginMigration; import com.loohp.imageframe.migration.PluginMigrationRegistry; @@ -58,45 +152,11 @@ import com.loohp.imageframe.utils.MathUtils; import com.loohp.imageframe.utils.PlayerUtils; import com.loohp.platformscheduler.Scheduler; + import net.kyori.adventure.text.Component; import net.kyori.adventure.text.event.ClickEvent; import net.kyori.adventure.text.format.NamedTextColor; import net.md_5.bungee.api.ChatColor; -import org.bukkit.Bukkit; -import org.bukkit.Material; -import org.bukkit.OfflinePlayer; -import org.bukkit.World; -import org.bukkit.command.Command; -import org.bukkit.command.CommandExecutor; -import org.bukkit.command.CommandSender; -import org.bukkit.command.TabCompleter; -import org.bukkit.entity.Player; -import org.bukkit.inventory.Inventory; -import org.bukkit.inventory.ItemStack; -import org.bukkit.map.MapCursor; -import org.bukkit.map.MapView; - -import java.io.IOException; -import java.net.URLConnection; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Comparator; -import java.util.Date; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.PrimitiveIterator; -import java.util.Set; -import java.util.UUID; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Stream; - -import static com.loohp.imageframe.language.TranslationKey.*; -import static com.loohp.imageframe.utils.CommandSenderUtils.sendMessage; -import static com.loohp.imageframe.utils.ComponentUtils.translatable; -import static net.kyori.adventure.text.Component.text; public class Commands implements CommandExecutor, TabCompleter { @@ -243,7 +303,7 @@ public boolean onCommand(CommandSender sender, Command cmd, String label, String if (ImageFrame.imageUploadManager.isOperational() && url.equalsIgnoreCase("upload")) { UUID user = isConsole ? ImageMap.CONSOLE_CREATOR : player.getUniqueId(); PendingUpload pendingUpload = ImageFrame.imageUploadManager.newPendingUpload(user); - String uploadUrl = pendingUpload.getUrl(ImageFrame.uploadServiceDisplayURL, user); + String uploadUrl = pendingUpload.getUrl(ImageFrame.uploadServiceBaseURI, user); Scheduler.runTaskLaterAsynchronously(ImageFrame.plugin, () -> sendMessage(sender, translatable(UPLOAD_LINK, Component.text(uploadUrl).color(NamedTextColor.YELLOW).clickEvent(ClickEvent.openUrl(uploadUrl))).color(NamedTextColor.GREEN)), 2); url = pendingUpload.getFileBlocking().toURI().toURL().toString(); } @@ -398,7 +458,7 @@ public boolean onCommand(CommandSender sender, Command cmd, String label, String if (ImageFrame.imageUploadManager.isOperational() && url.equalsIgnoreCase("upload")) { UUID user = player.getUniqueId(); PendingUpload pendingUpload = ImageFrame.imageUploadManager.newPendingUpload(user); - String uploadUrl = pendingUpload.getUrl(ImageFrame.uploadServiceDisplayURL, user); + String uploadUrl = pendingUpload.getUrl(ImageFrame.uploadServiceBaseURI, user); Scheduler.runTaskLaterAsynchronously(ImageFrame.plugin, () -> sendMessage(sender, translatable(UPLOAD_LINK, Component.text(uploadUrl).color(NamedTextColor.YELLOW).clickEvent(ClickEvent.openUrl(uploadUrl))).color(NamedTextColor.GREEN)), 2); url = pendingUpload.getFileBlocking().toURI().toURL().toString(); } @@ -846,7 +906,7 @@ public boolean onCommand(CommandSender sender, Command cmd, String label, String if (ImageFrame.imageUploadManager.isOperational() && urlImageMap.getUrl().equalsIgnoreCase("upload")) { UUID user = !(sender instanceof Player) ? ImageMap.CONSOLE_CREATOR : ((Player) sender).getUniqueId(); PendingUpload pendingUpload = ImageFrame.imageUploadManager.newPendingUpload(user); - String uploadUrl = pendingUpload.getUrl(ImageFrame.uploadServiceDisplayURL, user); + String uploadUrl = pendingUpload.getUrl(ImageFrame.uploadServiceBaseURI, user); Scheduler.runTaskLaterAsynchronously(ImageFrame.plugin, () -> sendMessage(sender, translatable(UPLOAD_LINK, Component.text(uploadUrl).color(NamedTextColor.YELLOW).clickEvent(ClickEvent.openUrl(uploadUrl))).color(NamedTextColor.GREEN)), 2); String newUrl = pendingUpload.getFileBlocking().toURI().toURL().toString(); if (HTTPRequestUtils.getContentSize(newUrl) > ImageFrame.maxImageFileSize) { diff --git a/common/src/main/java/com/loohp/imageframe/ImageFrame.java b/common/src/main/java/com/loohp/imageframe/ImageFrame.java index a2f7d4b..eafe2ba 100644 --- a/common/src/main/java/com/loohp/imageframe/ImageFrame.java +++ b/common/src/main/java/com/loohp/imageframe/ImageFrame.java @@ -20,6 +20,27 @@ package com.loohp.imageframe; +import java.io.File; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URL; +import java.text.SimpleDateFormat; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import javax.imageio.ImageIO; +import javax.imageio.spi.IIORegistry; + +import org.bukkit.Bukkit; +import org.bukkit.ChatColor; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.java.JavaPlugin; + import com.loohp.imageframe.config.Config; import com.loohp.imageframe.debug.Debug; import com.loohp.imageframe.invisibleframe.InvisibleFrameManager; @@ -56,25 +77,8 @@ import com.loohp.platformscheduler.ScheduledTask; import com.loohp.platformscheduler.Scheduler; import com.twelvemonkeys.imageio.plugins.webp.WebPImageReaderSpi; -import net.kyori.adventure.key.Key; -import org.bukkit.Bukkit; -import org.bukkit.ChatColor; -import org.bukkit.command.CommandSender; -import org.bukkit.entity.Player; -import org.bukkit.plugin.Plugin; -import org.bukkit.plugin.java.JavaPlugin; -import javax.imageio.ImageIO; -import javax.imageio.spi.IIORegistry; -import java.io.File; -import java.io.IOException; -import java.net.MalformedURLException; -import java.net.URL; -import java.text.SimpleDateFormat; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; +import net.kyori.adventure.key.Key; public class ImageFrame extends JavaPlugin { @@ -124,9 +128,7 @@ public class ImageFrame extends JavaPlugin { public static boolean combinedByDefault; public static boolean uploadServiceEnabled; - public static String uploadServiceDisplayURL; - public static String uploadServiceServerAddress; - public static int uploadServiceServerPort; + public static URI uploadServiceBaseURI; public static boolean imageFrameClientEnabled; @@ -151,12 +153,10 @@ public class ImageFrame extends JavaPlugin { public static CustomClientNetworkManager customClientNetworkManager; public static boolean isURLAllowed(String link) { - if (!restrictImageUrlEnabled) { - return true; - } - if ("upload".equals(link) || imageUploadManager.wasUploaded(link)) { + if (!restrictImageUrlEnabled || "upload".equals(link) || imageUploadManager.wasUploaded(link)) { return true; } + try { URL url = new URL(link); return restrictImageUrls.stream().anyMatch(whitelisted -> { @@ -280,7 +280,7 @@ public void onEnable() { rateLimitedPacketSendingManager = new RateLimitedPacketSendingManager(); invisibleFrameManager = new InvisibleFrameManager(); imageMapCreationTaskManager = new ImageMapCreationTaskManager(ImageFrame.parallelProcessingLimit); - imageUploadManager = new ImageUploadManager(uploadServiceEnabled, uploadServiceServerAddress, uploadServiceServerPort); + imageUploadManager = new ImageUploadManager(uploadServiceEnabled, uploadServiceBaseURI); customClientNetworkManager = new CustomClientNetworkManager(imageFrameClientEnabled); if (isPluginEnabled("PlaceholderAPI")) { @@ -374,9 +374,7 @@ public void reloadConfig() { combinedByDefault = config.getConfiguration().getBoolean("Settings.CombinedByDefault"); uploadServiceEnabled = config.getConfiguration().getBoolean("UploadService.Enabled"); - uploadServiceDisplayURL = config.getConfiguration().getString("UploadService.DisplayURL"); - uploadServiceServerAddress = config.getConfiguration().getString("UploadService.WebServer.Host"); - uploadServiceServerPort = config.getConfiguration().getInt("UploadService.WebServer.Port"); + uploadServiceBaseURI = URI.create(config.getConfiguration().getString("UploadService.BaseURI")); imageFrameClientEnabled = config.getConfiguration().getBoolean("ImageFrameClient.Enabled"); diff --git a/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java b/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java index 30fe504..f1a217a 100644 --- a/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java +++ b/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java @@ -20,314 +20,345 @@ package com.loohp.imageframe.upload; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.RemovalNotification; -import com.loohp.imageframe.ImageFrame; -import com.loohp.imageframe.utils.FileUtils; -import com.loohp.imageframe.utils.JarUtils; -import com.loohp.imageframe.utils.SizeLimitedByteArrayOutputStream; -import com.loohp.platformscheduler.Scheduler; -import com.sun.net.httpserver.HttpExchange; -import com.sun.net.httpserver.HttpHandler; -import com.sun.net.httpserver.HttpServer; -import com.twelvemonkeys.net.MIMEUtil; -import org.apache.commons.fileupload.MultipartStream; - import java.io.File; import java.io.IOException; -import java.io.InputStream; -import java.net.BindException; -import java.net.InetSocketAddress; -import java.net.URL; +import java.net.URI; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.Arrays; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; -import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; -public class ImageUploadManager implements AutoCloseable { +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.RemovalNotification; +import com.loohp.imageframe.ImageFrame; +import com.loohp.imageframe.utils.FileUtils; +import com.loohp.platformscheduler.Scheduler; +import io.netty.bootstrap.ServerBootstrap; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.channel.Channel; +import io.netty.channel.ChannelFutureListener; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInitializer; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.MultiThreadIoEventLoopGroup; +import io.netty.channel.SimpleChannelInboundHandler; +import io.netty.channel.nio.NioIoHandler; +import io.netty.channel.socket.SocketChannel; +import io.netty.channel.socket.nio.NioServerSocketChannel; +import io.netty.handler.codec.http.DefaultFullHttpResponse; +import io.netty.handler.codec.http.DefaultHttpResponse; +import io.netty.handler.codec.http.FullHttpRequest; +import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpObjectAggregator; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.HttpServerCodec; +import io.netty.handler.codec.http.HttpVersion; +import io.netty.handler.codec.http.LastHttpContent; +import io.netty.handler.codec.http.QueryStringDecoder; +import io.netty.handler.codec.http.multipart.DefaultHttpDataFactory; +import io.netty.handler.codec.http.multipart.FileUpload; +import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder; +import io.netty.handler.codec.http.multipart.InterfaceHttpData; +import io.netty.handler.stream.ChunkedNioFile; +import io.netty.handler.stream.ChunkedWriteHandler; + +public class ImageUploadManager implements AutoCloseable { public static final long EXPIRATION = TimeUnit.MINUTES.toMillis(5); - private final HttpServer server; + private final URI uri; + private final String pathPrefix; + private final File webRootDir; private final File uploadDir; + private final Map pendingUploads; private final AtomicLong imagesUploadedCounter; - public ImageUploadManager(boolean enabled, String host, int port) { + private EventLoopGroup bossGroup; + private EventLoopGroup workerGroup; + private Channel serverChannel; + + public ImageUploadManager(boolean enabled, URI uri) { + this.uri = uri; + this.pathPrefix = normalizePrefix(uri.getPath()); + this.webRootDir = new File(ImageFrame.plugin.getDataFolder(), "upload/web"); this.uploadDir = new File(ImageFrame.plugin.getDataFolder(), "upload/images"); this.imagesUploadedCounter = new AtomicLong(0); + FileUtils.removeFolderRecursively(uploadDir); + if (!uploadDir.exists()) { + uploadDir.mkdirs(); + } + + if (!webRootDir.exists()) { + webRootDir.mkdirs(); + } + Cache cache = CacheBuilder.newBuilder() .expireAfterAccess(EXPIRATION, TimeUnit.MILLISECONDS) .removalListener((RemovalNotification notification) -> { - if (notification.wasEvicted()) { - PendingUpload pendingUpload = notification.getValue(); - if (pendingUpload != null) { - pendingUpload.getFuture().completeExceptionally(new LinkTimeoutException()); - } + if (notification.wasEvicted() && notification.getValue() != null) { + notification.getValue().getFuture().completeExceptionally(new LinkTimeoutException()); } }) .build(); - this.pendingUploads = cache.asMap(); - HttpServer server = null; - try { - FileUtils.removeFolderRecursively(uploadDir); - if (!uploadDir.exists()) { - uploadDir.mkdirs(); - } - if (!webRootDir.exists()) { - webRootDir.mkdirs(); - } - JarUtils.copyFolderFromJar("upload/web", ImageFrame.plugin.getDataFolder(), JarUtils.CopyOption.COPY_IF_NOT_EXIST); - if (enabled) { - System.setProperty("sun.net.httpserver.maxReqTime", "30"); - System.setProperty("sun.net.httpserver.maxRspTime", "30"); - server = HttpServer.create(new InetSocketAddress(host, port), 8); - server.createContext("/", new FileHandler()); - server.createContext("/upload", new UploadHandler()); - server.setExecutor(Executors.newFixedThreadPool(8)); - server.start(); - } - } catch (BindException e) { - new RuntimeException("Unable to start ImageFrame upload server (Perhaps port + " + port + " is already used by another program?)", e).printStackTrace(); - } catch (IOException e) { - new RuntimeException("Unable to start ImageFrame upload server", e).printStackTrace(); - } - this.server = server; - } - - public PendingUpload newPendingUpload(UUID user) { - PendingUpload existing = pendingUploads.remove(user); - if (existing != null) { - existing.getFuture().completeExceptionally(new LinkTimeoutException()); + this.pendingUploads = cache.asMap(); + if (enabled) { + startServer(); } - PendingUpload pendingUpload = PendingUpload.create(EXPIRATION); - pendingUploads.put(user, pendingUpload); - return pendingUpload; } - public AtomicLong getImagesUploadedCounter() { - return imagesUploadedCounter; - } + private void startServer() { + bossGroup = new MultiThreadIoEventLoopGroup(1, NioIoHandler.newFactory()); + workerGroup = new MultiThreadIoEventLoopGroup(0, NioIoHandler.newFactory()); // 0 = Netty default (2 * cores) - public boolean wasUploaded(String url) { try { - File file = Paths.get(new URL(url).toURI()).toFile(); - return uploadDir.equals(file.getParentFile()); - } catch (Exception e) { - return false; + ServerBootstrap bootstrap = new ServerBootstrap(); + bootstrap.group(bossGroup, workerGroup) + .channelFactory(NioServerSocketChannel::new) + .childHandler(new ChannelInitializer() { + @Override + protected void initChannel(SocketChannel ch) { + ch.pipeline().addLast(new HttpServerCodec()); + ch.pipeline().addLast(new HttpObjectAggregator((int) ImageFrame.maxImageFileSize)); + ch.pipeline().addLast(new ChunkedWriteHandler()); + ch.pipeline().addLast(new UploadServerHandler()); + } + }); + + // Default to localhost:80 if no config option is set. + int port = uri.getPort() == -1 ? 80 : uri.getPort(); + String host = uri.getHost() == null ? "127.0.0.1" : uri.getHost(); + + serverChannel = bootstrap.bind(host, port).sync().channel(); + } catch (InterruptedException error) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Failed to start Netty upload server", error); } } - public boolean isOperational() { - return server != null; - } + private String normalizePrefix(String path) { + if (path == null || path.trim().isEmpty() || path.equals("/")) { + return ""; + } - @Override - public void close() { - if (server != null) { - server.stop(0); + if (!path.startsWith("/")) { + path = "/" + path; } + + if (path.endsWith("/")) { + path = path.substring(0, path.length() - 1); + } + + return path; } - private class FileHandler implements HttpHandler { + private class UploadServerHandler extends SimpleChannelInboundHandler { @Override - public void handle(HttpExchange exchange) throws IOException { - try { - if (!exchange.getRequestMethod().equalsIgnoreCase("GET")) { - exchange.sendResponseHeaders(405, -1); // Method Not Allowed + protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest request) throws Exception { + if (!request.decoderResult().isSuccess()) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\": \"Bad request\""); + return; + } + + QueryStringDecoder decoder = new QueryStringDecoder(request.uri()); + String uriPath = decoder.path(); + + if (!pathPrefix.isEmpty()) { + if (!uriPath.startsWith(pathPrefix)) { + send(ctx, HttpResponseStatus.NOT_FOUND, "{\"error\": \"Not found\""); return; } - if (!webRootDir.exists()) { - webRootDir.mkdirs(); - } - File file = resolvePath(webRootDir.toPath().toAbsolutePath(), Paths.get("." + exchange.getRequestURI().getPath())).toFile(); - File targetFile; - byte[] bytes; - if (file.exists()) { - if (file.isDirectory()) { - targetFile = new File(file, "index.html"); - bytes = Files.readAllBytes(targetFile.toPath()); - } else { - targetFile = file; - bytes = Files.readAllBytes(file.toPath()); - } - } else { - targetFile = new File(webRootDir, "index.html"); - bytes = Files.readAllBytes(new File(webRootDir, "index.html").toPath()); - } - String targetFileName = targetFile.getName(); - String mimeType; - if (targetFileName.contains(".")) { - String targetFileExtension = targetFileName.substring(targetFileName.lastIndexOf(".") + 1); - mimeType = MIMEUtil.getMIMEType(targetFileExtension); - } else { - mimeType = null; + + uriPath = uriPath.substring(pathPrefix.length()); + if (uriPath.isEmpty()) { + uriPath = "/"; } - exchange.getResponseHeaders().set("Content-Type", mimeType == null ? "text/plain" : mimeType); - exchange.sendResponseHeaders(200, bytes.length); - exchange.getResponseBody().write(bytes); - exchange.getResponseBody().close(); - } finally { - exchange.close(); } - } - private Path resolvePath(Path baseDirPath, Path userPath) { - Path resolvedPath = baseDirPath.resolve(userPath).normalize(); - if (!resolvedPath.startsWith(baseDirPath)) { - return baseDirPath; + if (request.method() == HttpMethod.GET) { + handleStatic(ctx, uriPath); + } else if (request.method() == HttpMethod.POST && uriPath.equals("/upload")) { + handleUpload(ctx, request, decoder); + } else { + send(ctx, HttpResponseStatus.METHOD_NOT_ALLOWED, "\"error\": \"Method not allowed\""); } - return resolvedPath; } } - private class UploadHandler implements HttpHandler { - @Override - public void handle(HttpExchange exchange) throws IOException { - try { - if (!exchange.getRequestMethod().equalsIgnoreCase("POST")) { - exchange.sendResponseHeaders(405, -1); - return; - } + private void handleStatic(ChannelHandlerContext ctx, String path) throws IOException { + Path base = webRootDir.toPath().toAbsolutePath(); + Path resolved = base.resolve("." + path).normalize(); + + if (!resolved.startsWith(base) || !Files.exists(resolved)) { + resolved = base.resolve("index.html"); + } - // Extract query parameters - String query = exchange.getRequestURI().getQuery(); - Map queryParams = parseQueryParams(query); - String user = queryParams.get("user"); - String id = queryParams.get("id"); + File file = resolved.toFile(); + long fileLength = file.length(); - PendingUpload pendingUpload = findPendingUpload(user, id); - if (pendingUpload == null) { - sendResponse(exchange, 400, "{\"error\":\"Invalid or missing user or id\"}"); - return; - } + HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK); + response.headers().set(HttpHeaderNames.CONTENT_LENGTH, fileLength); + response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/html"); - List contentType = Arrays.asList(exchange.getRequestHeaders().getFirst("Content-Type").split(";")); - if (!contentType.get(0).trim().equals("multipart/form-data")) { - sendResponse(exchange, 400, "{\"error\":\"Invalid content type\"}"); - return; - } - byte[] boundary = contentType.stream() - .map(s -> s.trim()) - .filter(s -> s.startsWith("boundary=")) - .findFirst() - .map(s -> s.substring("boundary=".length()).getBytes(StandardCharsets.UTF_8)) - .orElse(null); - if (boundary == null) { - sendResponse(exchange, 400, "{\"error\":\"Invalid multipart boundary\"}"); - return; - } + ctx.write(response); - SizeLimitedByteArrayOutputStream output = new SizeLimitedByteArrayOutputStream(ImageFrame.maxImageFileSize); - - try (InputStream inputStream = exchange.getRequestBody()) { - MultipartStream multipartStream = new MultipartStream(inputStream, boundary, 2048, null); - boolean nextPart = multipartStream.skipPreamble(); - while (nextPart) { - List headers = Arrays.asList(multipartStream.readHeaders().split(";")); - String name = headers.stream() - .map(s -> s.trim()).filter(s -> s.startsWith("name=")) - .findFirst() - .map(s -> s.substring("name=".length())) - .orElse(null); - if ("image".equals(name) || "\"image\"".equals(name)) { - multipartStream.readBodyData(output); - break; - } else { - multipartStream.discardBodyData(); - nextPart = multipartStream.readBoundary(); - } - } + // Use ChunkedNioFile for better NIO integration + ctx.write(new ChunkedNioFile(file)); + ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT).addListener(ChannelFutureListener.CLOSE); + } + + private void handleUpload(ChannelHandlerContext ctx, FullHttpRequest request, QueryStringDecoder decoder) + throws Exception { + + Map> params = decoder.parameters(); + + String user = getParam(params, "user"); + String id = getParam(params, "id"); + + PendingUpload pending = findPendingUpload(user, id); + if (pending == null) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\": \"Invalid upload token\""); + return; + } + + HttpPostRequestDecoder postDecoder = new HttpPostRequestDecoder(new DefaultHttpDataFactory(false), request); + + try { + for (InterfaceHttpData data : postDecoder.getBodyHttpDatas()) { + if (!(data instanceof FileUpload)) { + continue; } - byte[] fileData = output.toByteArray(); - // Ensure upload directory exists - if (!uploadDir.exists()) { - uploadDir.mkdir(); + FileUpload upload = (FileUpload) data; + if (!upload.isCompleted()) { + continue; } - // Save the file with UUID as the filename + byte[] fileData = new byte[(int) upload.length()]; + upload.getByteBuf().readBytes(fileData); + File outputFile = new File(uploadDir, id + ".png"); Files.write(outputFile.toPath(), fileData); + imagesUploadedCounter.incrementAndGet(); pendingUploads.remove(UUID.fromString(user)); - pendingUpload.getFuture().complete(outputFile); - Scheduler.runTaskLaterAsynchronously(ImageFrame.plugin, () -> outputFile.delete(), EXPIRATION / 50); + pending.getFuture().complete(outputFile); - // Send response - sendResponse(exchange, 200, "{\"message\":\"File uploaded successfully\"}"); - } finally { - exchange.close(); + Scheduler.runTaskLaterAsynchronously(ImageFrame.plugin, outputFile::delete, EXPIRATION / 50); + send(ctx, HttpResponseStatus.OK, "{\"message\": \"File uploaded successfully\"}"); + return; } - } - // Parse query parameters from URL - private Map parseQueryParams(String query) { - Map map = new HashMap<>(); - if (query != null) { - String[] pairs = query.split("&"); - for (String pair : pairs) { - String[] keyValue = pair.split("="); - if (keyValue.length == 2) { - map.put(keyValue[0], keyValue[1]); - } - } - } - return map; + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\": \"Missing image field\""); + } finally { + postDecoder.destroy(); } + } - private PendingUpload findPendingUpload(String user, String id) { - try { - PendingUpload pendingUpload = pendingUploads.get(UUID.fromString(user)); - if (!pendingUpload.getId().equals(UUID.fromString(id))) { - return null; - } - return pendingUpload; - } catch (Exception e) { + private String getParam(Map> map, String key) { + List values = map.get(key); + return (values == null || values.isEmpty()) ? null : values.get(0); + } + + private PendingUpload findPendingUpload(String user, String id) { + try { + UUID userUUID = UUID.fromString(user); + UUID idUUID = UUID.fromString(id); + + PendingUpload pending = pendingUploads.get(userUUID); + if (pending == null || !pending.getId().equals(idUUID)) { return null; } + + return pending; + } catch (Exception e) { + return null; + } + } + + private void send(ChannelHandlerContext ctx, HttpResponseStatus status, String msg) { + ByteBuf buf = Unpooled.copiedBuffer(msg, StandardCharsets.UTF_8); + FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, status, buf); + + response.headers().set(HttpHeaderNames.CONTENT_LENGTH, buf.readableBytes()); + response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/plain"); + ctx.writeAndFlush(response).addListener(ChannelFutureListener.CLOSE); + } + + public boolean wasUploaded(String url) { + try { + // Parse the URL to get the file path + URI uri = new URI(url); + String path = uri.getPath(); + + // Extract filename from the path + String filename = path.substring(path.lastIndexOf('/') + 1); + + // Check if the file exists in the upload directory + File file = new File(uploadDir, filename); + + // Verify the file exists and is in the upload directory + return file.exists() && uploadDir.equals(file.getParentFile()); + } catch (Exception e) { + return false; } + } - // Send JSON response - private void sendResponse(HttpExchange exchange, int statusCode, String message) throws IOException { - exchange.getResponseHeaders().set("Content-Type", "application/json"); - exchange.sendResponseHeaders(statusCode, message.length()); - exchange.getResponseBody().write(message.getBytes()); - exchange.getResponseBody().close(); + public PendingUpload newPendingUpload(UUID user) { + PendingUpload existing = pendingUploads.remove(user); + if (existing != null) { + existing.getFuture().completeExceptionally(new LinkTimeoutException()); } + + PendingUpload pendingUpload = PendingUpload.create(EXPIRATION); + pendingUploads.put(user, pendingUpload); + return pendingUpload; } - public static class LinkTimeoutException extends Exception { + public boolean isOperational() { + return serverChannel != null && serverChannel.isActive(); + } - public LinkTimeoutException() { - super(); + public AtomicLong getImagesUploadedCounter() { + return imagesUploadedCounter; + } + + @Override + public void close() { + if (serverChannel != null) { + serverChannel.close(); } - public LinkTimeoutException(String message) { - super(message); + if (bossGroup != null) { + bossGroup.shutdownGracefully(); } - public LinkTimeoutException(String message, Throwable cause) { - super(message, cause); + if (workerGroup != null) { + workerGroup.shutdownGracefully(); } + } - public LinkTimeoutException(Throwable cause) { - super(cause); + public static class LinkTimeoutException extends Exception { + public LinkTimeoutException() { + super(); } + public LinkTimeoutException(String msg) { + super(msg); + } } -} +} \ No newline at end of file diff --git a/common/src/main/java/com/loohp/imageframe/upload/PendingUpload.java b/common/src/main/java/com/loohp/imageframe/upload/PendingUpload.java index ee94844..64474c7 100644 --- a/common/src/main/java/com/loohp/imageframe/upload/PendingUpload.java +++ b/common/src/main/java/com/loohp/imageframe/upload/PendingUpload.java @@ -20,15 +20,16 @@ package com.loohp.imageframe.upload; -import org.bukkit.Bukkit; - import java.io.File; +import java.net.URI; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import org.bukkit.Bukkit; + public class PendingUpload { public static PendingUpload create(long timeToLive) { @@ -65,12 +66,44 @@ public File getFileBlocking() throws InterruptedException, ImageUploadManager.Li } } - public String getUrl(String domain, UUID user) { + /** + * Get the upload URL for this pending upload + * + * @param baseUri The base URI of the upload server (e.g., + * http://localhost:8080/images) + * @param user The user UUID + * @return The complete URL with query parameters + */ + public String getUrl(URI baseUri, UUID user) { + String name = Bukkit.getOfflinePlayer(user).getName(); + StringBuilder query = new StringBuilder(); + query.append("user=").append(user) + .append("&id=").append(id) + .append("&expire=").append(expire); + + if (name != null) { + query.append("&name=").append(name); + } + + // Ensure base path ends with /upload + String basePath = baseUri.toString(); + if (!basePath.endsWith("/")) { + basePath += "/"; + } + + return basePath + "upload?" + query.toString(); + } + + /** + * Legacy method for backward compatibility + */ + @Deprecated + public String getUrl(String webAddress, UUID user) { String name = Bukkit.getOfflinePlayer(user).getName(); if (name == null) { - return domain + "?user=" + user + "&id=" + id + "&expire=" + expire; + return webAddress + "?user=" + user + "&id=" + id + "&expire=" + expire; } else { - return domain + "?user=" + user + "&id=" + id + "&expire=" + expire + "&name=" + name; + return webAddress + "?user=" + user + "&id=" + id + "&expire=" + expire + "&name=" + name; } } } \ No newline at end of file diff --git a/common/src/main/resources/config.yml b/common/src/main/resources/config.yml index a495e6c..0b296f0 100644 --- a/common/src/main/resources/config.yml +++ b/common/src/main/resources/config.yml @@ -75,16 +75,13 @@ Settings: UploadService: #Changing this value requires a restart Enabled: true - #This is only used for displaying to the end user - DisplayURL: "http://change.this.to.your.server.ip.in.the.config" - WebServer: - #Address in which the webserver binds to, defaults to 0.0.0.0 - #Use 127.0.0.1 to limit to local access - #Changing this value requires a restart - Host: "0.0.0.0" - #Port in which the webserver is hosted, make sure it is not blocked by your firewall - #Changing this value requires a restart - Port: 8517 + # This is the base URL the plugin uses. Below are some common examples: + # http://localhost/ + # http://127.0.0.1:8080/ + # http://example.com/ + # http://example.com/image_frame/ + # By default we listen on localhost, port 80. + BaseURI: "http://localhost/" #ImageFrame's complimentary mod where you can see images in its native resolution in full color ImageFrameClient: From c6dea9a95c16ce96b99664d7202e5d809d02b95c Mon Sep 17 00:00:00 2001 From: Lunar Starstrum Date: Sun, 1 Mar 2026 16:25:20 -0600 Subject: [PATCH 2/3] Fix several vulnerabilities --- .../imageframe/upload/ImageUploadManager.java | 266 +++++++++++++++--- 1 file changed, 233 insertions(+), 33 deletions(-) diff --git a/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java b/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java index f1a217a..2e6520d 100644 --- a/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java +++ b/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java @@ -20,8 +20,12 @@ package com.loohp.imageframe.upload; +import java.awt.Graphics2D; +import java.awt.image.BufferedImage; import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.URI; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -32,6 +36,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import javax.imageio.ImageIO; + import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.common.cache.RemovalNotification; @@ -73,6 +79,15 @@ import io.netty.handler.stream.ChunkedWriteHandler; public class ImageUploadManager implements AutoCloseable { + private static final byte[] PNG_SIGNATURE = new byte[] { + (byte) 0x89, 0x50, 0x4E, 0x47, + 0x0D, 0x0A, 0x1A, 0x0A + }; + + private static final int MAX_WIDTH = 8192; + private static final int MAX_HEIGHT = 8192; + private static final long MAX_PIXELS = 8192L * 8192L; + public static final long EXPIRATION = TimeUnit.MINUTES.toMillis(5); private final URI uri; @@ -169,54 +184,143 @@ private class UploadServerHandler extends SimpleChannelInboundHandler= 0) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Invalid path\"}"); return; } - QueryStringDecoder decoder = new QueryStringDecoder(request.uri()); - String uriPath = decoder.path(); + // Reject Windows-style separators to prevent bypass tricks + if (rawPath.contains("\\")) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Invalid path\"}"); + return; + } + // Normalize dot segments safely using URI + String normalizedPath; + try { + normalizedPath = new URI(null, null, rawPath, null).normalize().getPath(); + } catch (Exception e) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Invalid URI\"}"); + return; + } + + if (normalizedPath == null || normalizedPath.isEmpty()) { + normalizedPath = "/"; + } + + // Apply prefix restriction if (!pathPrefix.isEmpty()) { - if (!uriPath.startsWith(pathPrefix)) { - send(ctx, HttpResponseStatus.NOT_FOUND, "{\"error\": \"Not found\""); + + if (!normalizedPath.startsWith(pathPrefix)) { + send(ctx, HttpResponseStatus.NOT_FOUND, "{\"error\":\"Not found\"}"); return; } - uriPath = uriPath.substring(pathPrefix.length()); - if (uriPath.isEmpty()) { - uriPath = "/"; + normalizedPath = normalizedPath.substring(pathPrefix.length()); + + if (normalizedPath.isEmpty()) { + normalizedPath = "/"; } } - if (request.method() == HttpMethod.GET) { - handleStatic(ctx, uriPath); - } else if (request.method() == HttpMethod.POST && uriPath.equals("/upload")) { + // At this point: + // - Path is normalized + // - No backslashes + // - No null bytes + // - Prefix stripped + // - Dot segments removed + + if (request.method() == HttpMethod.GET || request.method() == HttpMethod.HEAD) { + handleStatic(ctx, request, normalizedPath); + return; + } + + if (request.method() == HttpMethod.POST && "/upload".equals(normalizedPath)) { handleUpload(ctx, request, decoder); - } else { - send(ctx, HttpResponseStatus.METHOD_NOT_ALLOWED, "\"error\": \"Method not allowed\""); + return; } + + send(ctx, HttpResponseStatus.METHOD_NOT_ALLOWED, "{\"error\":\"Method not allowed\"}"); } } - private void handleStatic(ChannelHandlerContext ctx, String path) throws IOException { - Path base = webRootDir.toPath().toAbsolutePath(); - Path resolved = base.resolve("." + path).normalize(); + private void handleStatic(ChannelHandlerContext ctx, FullHttpRequest request, String normalizedPath) + throws IOException { + HttpMethod method = request.method(); + if (method != HttpMethod.GET && method != HttpMethod.HEAD) { + send(ctx, HttpResponseStatus.METHOD_NOT_ALLOWED, "{\"error\":\"Method not allowed\"}"); + return; + } + + // Resolve base directory canonically + Path basePath = webRootDir.getCanonicalFile().toPath(); + if (normalizedPath == null || normalizedPath.isEmpty()) { + normalizedPath = "/"; + } + + // Canonical check prevents traversal and symlink escape attacks + Path resolved = basePath.resolve("." + normalizedPath).normalize(); + Path canonical; + try { + canonical = resolved.toFile().getCanonicalFile().toPath(); + } catch (IOException e) { + send(ctx, HttpResponseStatus.NOT_FOUND, "{\"error\":\"Not found\"}"); + return; + } + + if (!canonical.startsWith(basePath)) { + send(ctx, HttpResponseStatus.FORBIDDEN, "{\"error\":\"Forbidden\"}"); + return; + } + + if (Files.isDirectory(canonical)) { + canonical = canonical.resolve("index.html"); + } - if (!resolved.startsWith(base) || !Files.exists(resolved)) { - resolved = base.resolve("index.html"); + if (!Files.exists(canonical) || !Files.isRegularFile(canonical)) { + canonical = basePath.resolve("index.html").toFile().getCanonicalFile().toPath(); + if (!canonical.startsWith(basePath) || !Files.exists(canonical)) { + send(ctx, HttpResponseStatus.NOT_FOUND, "{\"error\":\"Not found\"}"); + return; + } } - File file = resolved.toFile(); + File file = canonical.toFile(); long fileLength = file.length(); + String contentType = Files.probeContentType(canonical); + if (contentType == null) { + contentType = "application/octet-stream"; + } HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK); response.headers().set(HttpHeaderNames.CONTENT_LENGTH, fileLength); - response.headers().set(HttpHeaderNames.CONTENT_TYPE, "text/html"); + response.headers().set(HttpHeaderNames.CONTENT_TYPE, contentType); + + // Security Headers + response.headers().set("X-Content-Type-Options", "nosniff"); + response.headers().set("X-Frame-Options", "DENY"); + response.headers().set("Referrer-Policy", "no-referrer"); + + // Since this is upload UI, safest default is no-store + response.headers().set("Cache-Control", "no-store"); ctx.write(response); + if (method == HttpMethod.GET) { + ctx.write(new ChunkedNioFile(file)); + } - // Use ChunkedNioFile for better NIO integration - ctx.write(new ChunkedNioFile(file)); ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT).addListener(ChannelFutureListener.CLOSE); } @@ -224,17 +328,30 @@ private void handleUpload(ChannelHandlerContext ctx, FullHttpRequest request, Qu throws Exception { Map> params = decoder.parameters(); - String user = getParam(params, "user"); String id = getParam(params, "id"); PendingUpload pending = findPendingUpload(user, id); if (pending == null) { - send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\": \"Invalid upload token\""); + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Invalid upload token\"}"); + return; + } + + UUID userUUID; + UUID idUUID; + + try { + userUUID = UUID.fromString(user); + idUUID = UUID.fromString(id); + } catch (Exception e) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Invalid UUID format\"}"); return; } - HttpPostRequestDecoder postDecoder = new HttpPostRequestDecoder(new DefaultHttpDataFactory(false), request); + // Saves files smaller than 16KB in memory, writes bigger files to disk. + DefaultHttpDataFactory factory = new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE); + HttpPostRequestDecoder postDecoder = new HttpPostRequestDecoder(factory, request); + File tempFile = null; try { for (InterfaceHttpData data : postDecoder.getBodyHttpDatas()) { @@ -247,25 +364,108 @@ private void handleUpload(ChannelHandlerContext ctx, FullHttpRequest request, Qu continue; } - byte[] fileData = new byte[(int) upload.length()]; - upload.getByteBuf().readBytes(fileData); + // Enforce field name + if (!"image".equals(upload.getName())) { + continue; + } - File outputFile = new File(uploadDir, id + ".png"); - Files.write(outputFile.toPath(), fileData); + tempFile = upload.getFile(); + if (tempFile == null || !tempFile.exists()) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Upload failed\"}"); + return; + } - imagesUploadedCounter.incrementAndGet(); + // Validate PNG signature + if (!hasValidPngSignature(tempFile)) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Invalid PNG signature\"}"); + return; + } + + // Start re-encoding image to prevent attacks + BufferedImage decoded; + try (InputStream in = Files.newInputStream(tempFile.toPath())) { + decoded = ImageIO.read(in); + } + + if (decoded == null) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Invalid PNG format\"}"); + return; + } + + int width = decoded.getWidth(); + int height = decoded.getHeight(); + + // Enforce reasonable image dimension limits + if (width <= 0 || height <= 0 || width > MAX_WIDTH || height > MAX_HEIGHT + || ((long) width * height) > MAX_PIXELS) { + + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Image dimensions exceed limits\"}"); + return; + } + + // Normalize to safe ARGB + BufferedImage normalized = new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB); + Graphics2D g = normalized.createGraphics(); + g.drawImage(decoded, 0, 0, null); + g.dispose(); - pendingUploads.remove(UUID.fromString(user)); + // Atomic single-use token removal + if (!pendingUploads.remove(userUUID, pending)) { + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Token already used\"}"); + return; + } + + // Write clean PNG (re-encode strips malicious chunks) + File outputFile = new File(uploadDir, idUUID + ".png"); + try (OutputStream out = Files.newOutputStream(outputFile.toPath())) { + if (!ImageIO.write(normalized, "png", out)) { + send(ctx, HttpResponseStatus.INTERNAL_SERVER_ERROR, "{\"error\":\"Failed to encode PNG\"}"); + return; + } + } + + imagesUploadedCounter.incrementAndGet(); pending.getFuture().complete(outputFile); Scheduler.runTaskLaterAsynchronously(ImageFrame.plugin, outputFile::delete, EXPIRATION / 50); - send(ctx, HttpResponseStatus.OK, "{\"message\": \"File uploaded successfully\"}"); + send(ctx, HttpResponseStatus.OK, "{\"message\":\"File uploaded successfully\"}"); return; } - send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\": \"Missing image field\""); + send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Missing image field\"}"); } finally { + if (tempFile != null && tempFile.exists()) { + try { + Files.deleteIfExists(tempFile.toPath()); + } catch (IOException ignored) { + } + } + postDecoder.destroy(); + factory.cleanAllHttpData(); + } + } + + private boolean hasValidPngSignature(File file) { + if (file.length() < PNG_SIGNATURE.length) { + return false; + } + + try (InputStream in = Files.newInputStream(file.toPath())) { + byte[] header = new byte[PNG_SIGNATURE.length]; + if (in.read(header) != PNG_SIGNATURE.length) { + return false; + } + + for (int i = 0; i < PNG_SIGNATURE.length; i++) { + if (header[i] != PNG_SIGNATURE[i]) { + return false; + } + } + + return true; + } catch (IOException e) { + return false; } } From d7d099fa06cae28a346c5661a420d99295c37400 Mon Sep 17 00:00:00 2001 From: Lunar Starstrum Date: Sun, 1 Mar 2026 16:57:16 -0600 Subject: [PATCH 3/3] Add more configuration options --- .../java/com/loohp/imageframe/ImageFrame.java | 8 ++++++- .../imageframe/upload/ImageUploadManager.java | 22 +++++++++---------- common/src/main/resources/config.yml | 11 ++++++++++ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/common/src/main/java/com/loohp/imageframe/ImageFrame.java b/common/src/main/java/com/loohp/imageframe/ImageFrame.java index eafe2ba..3b0d7be 100644 --- a/common/src/main/java/com/loohp/imageframe/ImageFrame.java +++ b/common/src/main/java/com/loohp/imageframe/ImageFrame.java @@ -129,6 +129,9 @@ public class ImageFrame extends JavaPlugin { public static boolean uploadServiceEnabled; public static URI uploadServiceBaseURI; + public static long uploadServiceMaxCacheSize; + public static int uploadServiceMaxImageWidth; + public static int uploadServiceMaxImageHeight; public static boolean imageFrameClientEnabled; @@ -280,7 +283,7 @@ public void onEnable() { rateLimitedPacketSendingManager = new RateLimitedPacketSendingManager(); invisibleFrameManager = new InvisibleFrameManager(); imageMapCreationTaskManager = new ImageMapCreationTaskManager(ImageFrame.parallelProcessingLimit); - imageUploadManager = new ImageUploadManager(uploadServiceEnabled, uploadServiceBaseURI); + imageUploadManager = new ImageUploadManager(uploadServiceEnabled, uploadServiceBaseURI, uploadServiceMaxCacheSize, uploadServiceMaxImageWidth, uploadServiceMaxImageHeight); customClientNetworkManager = new CustomClientNetworkManager(imageFrameClientEnabled); if (isPluginEnabled("PlaceholderAPI")) { @@ -375,6 +378,9 @@ public void reloadConfig() { uploadServiceEnabled = config.getConfiguration().getBoolean("UploadService.Enabled"); uploadServiceBaseURI = URI.create(config.getConfiguration().getString("UploadService.BaseURI")); + uploadServiceMaxCacheSize = config.getConfiguration().getLong("UploadService.MaxCacheSize"); + uploadServiceMaxImageWidth = config.getConfiguration().getInt("UploadService.MaxImageWidth"); + uploadServiceMaxImageHeight = config.getConfiguration().getInt("UploadService.MaxImageHeight"); imageFrameClientEnabled = config.getConfiguration().getBoolean("ImageFrameClient.Enabled"); diff --git a/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java b/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java index 2e6520d..908c365 100644 --- a/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java +++ b/common/src/main/java/com/loohp/imageframe/upload/ImageUploadManager.java @@ -84,14 +84,13 @@ public class ImageUploadManager implements AutoCloseable { 0x0D, 0x0A, 0x1A, 0x0A }; - private static final int MAX_WIDTH = 8192; - private static final int MAX_HEIGHT = 8192; - private static final long MAX_PIXELS = 8192L * 8192L; - public static final long EXPIRATION = TimeUnit.MINUTES.toMillis(5); private final URI uri; private final String pathPrefix; + private final long maxCacheSize; + private final int maxWidth; + private final int maxHeight; private final File webRootDir; private final File uploadDir; @@ -103,9 +102,12 @@ public class ImageUploadManager implements AutoCloseable { private EventLoopGroup workerGroup; private Channel serverChannel; - public ImageUploadManager(boolean enabled, URI uri) { + public ImageUploadManager(boolean enabled, URI uri, long maxCacheSize, int maxImageWidth, int maxImageHeight) { this.uri = uri; this.pathPrefix = normalizePrefix(uri.getPath()); + this.maxCacheSize = maxCacheSize; + this.maxWidth = maxImageWidth; + this.maxHeight = maxImageHeight; this.webRootDir = new File(ImageFrame.plugin.getDataFolder(), "upload/web"); this.uploadDir = new File(ImageFrame.plugin.getDataFolder(), "upload/images"); @@ -190,7 +192,6 @@ protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest request) QueryStringDecoder decoder = new QueryStringDecoder(request.uri(), StandardCharsets.UTF_8); String rawPath = decoder.path(); - if (rawPath == null || rawPath.isEmpty()) { rawPath = "/"; } @@ -241,7 +242,6 @@ protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest request) // - No null bytes // - Prefix stripped // - Dot segments removed - if (request.method() == HttpMethod.GET || request.method() == HttpMethod.HEAD) { handleStatic(ctx, request, normalizedPath); return; @@ -348,8 +348,8 @@ private void handleUpload(ChannelHandlerContext ctx, FullHttpRequest request, Qu return; } - // Saves files smaller than 16KB in memory, writes bigger files to disk. - DefaultHttpDataFactory factory = new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE); + // Saves files smaller than 1MB (configurable) in memory, writes bigger files to disk. + DefaultHttpDataFactory factory = new DefaultHttpDataFactory(maxCacheSize); HttpPostRequestDecoder postDecoder = new HttpPostRequestDecoder(factory, request); File tempFile = null; @@ -396,8 +396,8 @@ private void handleUpload(ChannelHandlerContext ctx, FullHttpRequest request, Qu int height = decoded.getHeight(); // Enforce reasonable image dimension limits - if (width <= 0 || height <= 0 || width > MAX_WIDTH || height > MAX_HEIGHT - || ((long) width * height) > MAX_PIXELS) { + if (width <= 0 || height <= 0 || width > maxWidth || height > maxHeight + || ((long) width * height) > ((long) maxWidth * maxHeight)) { send(ctx, HttpResponseStatus.BAD_REQUEST, "{\"error\":\"Image dimensions exceed limits\"}"); return; diff --git a/common/src/main/resources/config.yml b/common/src/main/resources/config.yml index 0b296f0..255312d 100644 --- a/common/src/main/resources/config.yml +++ b/common/src/main/resources/config.yml @@ -82,6 +82,17 @@ UploadService: # http://example.com/image_frame/ # By default we listen on localhost, port 80. BaseURI: "http://localhost/" + # The maximum size of an image (in bytes) to have in memory before overflowing into the disk. + # This usually does not need to be changed unless if you have: + # - Large pictures being uploaded commonly + # - REALLY slow file io + # - Enormously large memory + # Defaults to 1MB (as in, images larger than 1MB will be temporarily saved to disk while images less than 1MB will be saved to memory). + MaxCacheSize: 1048576 + # The maximum amount of pixels allowed in the image's width. + MaxImageWidth: 8192 + # The maximum amount of pixels allowed in the image's height. + MaxImageHeight: 8192 #ImageFrame's complimentary mod where you can see images in its native resolution in full color ImageFrameClient: