From d06d3cabaf5b8a7dcd0dbfc584828d8e687e5dd5 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Wed, 24 Jun 2026 22:01:32 +0100 Subject: [PATCH] chore: update svg conversion and database import handling (#6796) --- .../software/common/util/SvgSanitizer.java | 5 +- .../common/util/SvgSanitizerTest.java | 47 +++++++++ .../software/SPDF/utils/SvgOverlayUtil.java | 4 + .../software/SPDF/utils/SvgToPdf.java | 20 +++- .../software/SPDF/utils/SvgToPdfTest.java | 98 +++++++++++++++++++ .../security/service/DatabaseService.java | 29 ++++++ .../service/DatabaseServiceMoreTest.java | 57 +++++++++++ 7 files changed, 254 insertions(+), 6 deletions(-) diff --git a/app/common/src/main/java/stirling/software/common/util/SvgSanitizer.java b/app/common/src/main/java/stirling/software/common/util/SvgSanitizer.java index c5addc0f32..35305b1d55 100644 --- a/app/common/src/main/java/stirling/software/common/util/SvgSanitizer.java +++ b/app/common/src/main/java/stirling/software/common/util/SvgSanitizer.java @@ -244,10 +244,7 @@ public class SvgSanitizer { return false; } - return normalized.startsWith("http://") - || normalized.startsWith("https://") - || normalized.startsWith("//") - || normalized.startsWith("file:"); + return true; } private boolean isUrlAllowed(String url) { diff --git a/app/common/src/test/java/stirling/software/common/util/SvgSanitizerTest.java b/app/common/src/test/java/stirling/software/common/util/SvgSanitizerTest.java index 433cf548d3..7b1c519a06 100644 --- a/app/common/src/test/java/stirling/software/common/util/SvgSanitizerTest.java +++ b/app/common/src/test/java/stirling/software/common/util/SvgSanitizerTest.java @@ -97,4 +97,51 @@ class SvgSanitizerTest { byte[] invalid = "not xml at all".getBytes(StandardCharsets.UTF_8); assertThrows(IOException.class, () -> sanitizer.sanitize(invalid)); } + + @Test + void testSanitize_removesRootRelativeLocalPath() throws IOException { + when(ssrfProtectionService.isUrlAllowed(anyString())).thenReturn(false); + String svg = + "" + + ""; + byte[] result = sanitizer.sanitize(svg.getBytes(StandardCharsets.UTF_8)); + String output = new String(result, StandardCharsets.UTF_8); + assertFalse(output.contains("/tmp/image.png"), "Root-relative local path must be stripped"); + } + + @Test + void testSanitize_removesRelativeLocalPath() throws IOException { + when(ssrfProtectionService.isUrlAllowed(anyString())).thenReturn(false); + String svg = + "" + + ""; + byte[] result = sanitizer.sanitize(svg.getBytes(StandardCharsets.UTF_8)); + String output = new String(result, StandardCharsets.UTF_8); + assertFalse(output.contains("assets/image.png"), "Relative local path must be stripped"); + } + + @Test + void testSanitize_removesRootRelativeWindowsDrivePath() throws IOException { + when(ssrfProtectionService.isUrlAllowed(anyString())).thenReturn(false); + String svg = + "" + + ""; + byte[] result = sanitizer.sanitize(svg.getBytes(StandardCharsets.UTF_8)); + String output = new String(result, StandardCharsets.UTF_8); + assertFalse( + output.contains("external-image"), "Root-relative Windows path must be stripped"); + } + + @Test + void testSanitize_keepsInDocumentFragmentReference() throws IOException { + String svg = + "" + + ""; + byte[] result = sanitizer.sanitize(svg.getBytes(StandardCharsets.UTF_8)); + String output = new String(result, StandardCharsets.UTF_8); + assertTrue( + output.contains("#gradient"), "In-document fragment references must be preserved"); + } } diff --git a/app/core/src/main/java/stirling/software/SPDF/utils/SvgOverlayUtil.java b/app/core/src/main/java/stirling/software/SPDF/utils/SvgOverlayUtil.java index b58bb055fc..2c7f708cc0 100644 --- a/app/core/src/main/java/stirling/software/SPDF/utils/SvgOverlayUtil.java +++ b/app/core/src/main/java/stirling/software/SPDF/utils/SvgOverlayUtil.java @@ -45,6 +45,10 @@ public class SvgOverlayUtil { @Override public void checkLoadExternalResource( ParsedURL resourceURL, ParsedURL docURL) { + // Inline data: URIs are self-contained (no network/file fetch). + if (resourceURL != null && "data".equals(resourceURL.getProtocol())) { + return; + } throw new SecurityException( "External resource loading is disabled for SVG overlays: " + resourceURL); diff --git a/app/core/src/main/java/stirling/software/SPDF/utils/SvgToPdf.java b/app/core/src/main/java/stirling/software/SPDF/utils/SvgToPdf.java index 54be5ca0ea..3d94df121f 100644 --- a/app/core/src/main/java/stirling/software/SPDF/utils/SvgToPdf.java +++ b/app/core/src/main/java/stirling/software/SPDF/utils/SvgToPdf.java @@ -19,6 +19,7 @@ import org.apache.batik.bridge.GVTBuilder; import org.apache.batik.bridge.UserAgent; import org.apache.batik.bridge.UserAgentAdapter; import org.apache.batik.gvt.GraphicsNode; +import org.apache.batik.util.ParsedURL; import org.apache.batik.util.XMLResourceDescriptor; import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.pdmodel.PDPage; @@ -63,7 +64,7 @@ public class SvgToPdf { } // 2. Build the GVT (Graphics Vector Tree) with timeout protection - UserAgent userAgent = new UserAgentAdapter(); + UserAgent userAgent = createSecureUserAgent(); DocumentLoader loader = new DocumentLoader(userAgent); BridgeContext ctx = new BridgeContext(userAgent, loader); ctx.setDynamicState(BridgeContext.DYNAMIC); @@ -94,6 +95,21 @@ public class SvgToPdf { } } + private UserAgent createSecureUserAgent() { + return new UserAgentAdapter() { + @Override + public void checkLoadExternalResource(ParsedURL resourceURL, ParsedURL docURL) { + // Inline data: URIs are self-contained (no network/file fetch) - allow them. + if (resourceURL != null && "data".equals(resourceURL.getProtocol())) { + return; + } + throw new SecurityException( + "External resource loading is disabled for SVG to PDF conversion: " + + resourceURL); + } + }; + } + private GraphicsNode buildGvtWithTimeout(BridgeContext ctx, SVGDocument svgDoc) throws IOException { GVTBuilder builder = new GVTBuilder(); @@ -202,7 +218,7 @@ public class SvgToPdf { svgDoc = factory.createSVGDocument("file:///input.svg", inputStream); } - UserAgent userAgent = new UserAgentAdapter(); + UserAgent userAgent = createSecureUserAgent(); DocumentLoader loader = new DocumentLoader(userAgent); BridgeContext ctx = new BridgeContext(userAgent, loader); ctx.setDynamicState(BridgeContext.DYNAMIC); diff --git a/app/core/src/test/java/stirling/software/SPDF/utils/SvgToPdfTest.java b/app/core/src/test/java/stirling/software/SPDF/utils/SvgToPdfTest.java index 7266efe220..63c3c73ffc 100644 --- a/app/core/src/test/java/stirling/software/SPDF/utils/SvgToPdfTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/utils/SvgToPdfTest.java @@ -2,12 +2,23 @@ package stirling.software.SPDF.utils; import static org.junit.jupiter.api.Assertions.*; +import java.awt.Color; +import java.awt.Graphics2D; +import java.awt.image.BufferedImage; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; +import java.util.Base64; import java.util.Collections; import java.util.List; +import javax.imageio.ImageIO; + +import org.apache.pdfbox.Loader; +import org.apache.pdfbox.pdmodel.PDDocument; +import org.apache.pdfbox.rendering.PDFRenderer; import org.junit.jupiter.api.Test; class SvgToPdfTest { @@ -124,4 +135,91 @@ class SvgToPdfTest { List svgs = Arrays.asList(null, null, new byte[0]); assertThrows(IOException.class, () -> SvgToPdf.combineIntoPdf(svgs)); } + + @Test + void convert_doesNotEmbedExternalFileResource() throws Exception { + Path external = Files.createTempFile("svg-external", ".png"); + BufferedImage red = new BufferedImage(100, 100, BufferedImage.TYPE_INT_RGB); + Graphics2D g = red.createGraphics(); + g.setColor(Color.RED); + g.fillRect(0, 0, 100, 100); + g.dispose(); + ImageIO.write(red, "png", external.toFile()); + + try { + String svg = + "" + + ""; + + byte[] pdf; + try { + pdf = SvgToPdf.convert(svg.getBytes(StandardCharsets.UTF_8)); + } catch (IOException blocked) { + return; + } + + try (PDDocument doc = Loader.loadPDF(pdf)) { + BufferedImage page = new PDFRenderer(doc).renderImageWithDPI(0, 72); + int rgb = page.getRGB(page.getWidth() / 2, page.getHeight() / 2); + int r = (rgb >> 16) & 0xff; + int gg = (rgb >> 8) & 0xff; + int b = rgb & 0xff; + assertFalse( + r > 200 && gg < 60 && b < 60, + "External file image must not be rendered into the PDF"); + } + } finally { + Files.deleteIfExists(external); + } + } + + // A self-contained SVG whose only content is an inline base64 data: image (a solid-red vector + // SVG). Vector data: images decode via batik-bridge without batik-codec, so this exercises the + // data: security allowance independent of raster codecs. + private static String svgWithInlineRedImage() { + String innerSvg = + "" + + ""; + String dataUri = + "data:image/svg+xml;base64," + + Base64.getEncoder() + .encodeToString(innerSvg.getBytes(StandardCharsets.UTF_8)); + return "" + + ""; + } + + @Test + void convert_rendersInlineDataUriImage() throws IOException { + byte[] pdf = SvgToPdf.convert(svgWithInlineRedImage().getBytes(StandardCharsets.UTF_8)); + try (PDDocument doc = Loader.loadPDF(pdf)) { + BufferedImage page = new PDFRenderer(doc).renderImageWithDPI(0, 72); + int rgb = page.getRGB(page.getWidth() / 2, page.getHeight() / 2); + int r = (rgb >> 16) & 0xff; + int gg = (rgb >> 8) & 0xff; + int b = rgb & 0xff; + assertTrue( + r > 200 && gg < 60 && b < 60, + "Inline data: image must be rendered into the PDF (center rgb=" + + Integer.toHexString(rgb) + + ")"); + } + } + + @Test + void combineIntoPdf_keepsPageWithInlineDataUriImage() throws IOException { + List svgs = + List.of( + SIMPLE_SVG.getBytes(StandardCharsets.UTF_8), + svgWithInlineRedImage().getBytes(StandardCharsets.UTF_8)); + byte[] pdf = SvgToPdf.combineIntoPdf(svgs); + try (PDDocument doc = Loader.loadPDF(pdf)) { + assertEquals(2, doc.getNumberOfPages(), "inline data: image page must not be dropped"); + } + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java index 2246903d2d..ffd9b5cb17 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/DatabaseService.java @@ -90,6 +90,20 @@ public class DatabaseService implements DatabaseServiceInterface { Pattern.compile("(?i)\\bFALSE\\b"), Pattern.compile("(?i)\\bNULL\\b")); + private static final java.util.List DENIED_PATTERNS = + java.util.List.of( + Pattern.compile("(?i)\\bCREATE\\s+(FORCE\\s+)?ALIAS\\b"), + Pattern.compile("(?i)\\bCREATE\\s+(FORCE\\s+)?TRIGGER\\b"), + Pattern.compile("(?i)\\bCREATE\\s+(FORCE\\s+)?AGGREGATE\\b"), + Pattern.compile("(?i)\\bCREATE\\s+LINKED\\s+TABLE\\b"), + Pattern.compile("(?i)\\bFILE_WRITE\\s*\\("), + Pattern.compile("(?i)\\bFILE_READ\\s*\\("), + Pattern.compile("(?i)\\bCSVWRITE\\s*\\("), + Pattern.compile("(?i)\\bCSVREAD\\s*\\("), + Pattern.compile("(?i)\\bLINK_SCHEMA\\s*\\("), + Pattern.compile("(?i)\\bRUNSCRIPT\\b"), + Pattern.compile("(?i)\\bSCRIPT\\s+TO\\b")); + private final ApplicationProperties.Datasource datasourceProps; private final DataSource dataSource; private final DatabaseNotificationServiceInterface backupNotificationService; @@ -508,6 +522,17 @@ public class DatabaseService implements DatabaseServiceInterface { String content = Files.readString(scriptPath); String normalizedContent = normalizeSqlContent(content); + String codeOnly = stripStringLiterals(normalizedContent); + for (Pattern deniedPattern : DENIED_PATTERNS) { + if (deniedPattern.matcher(codeOnly).find()) { + log.error( + "Blocked disallowed SQL in backup file matching: {}", + deniedPattern.pattern()); + throw new IllegalArgumentException( + "SQL script contains disallowed operations and was rejected."); + } + } + // Validate that content only contains allowed operations (whitelist approach) // Split by semicolons to check individual statements String[] statements = normalizedContent.split(";"); @@ -558,6 +583,10 @@ public class DatabaseService implements DatabaseServiceInterface { return sql.trim(); } + private String stripStringLiterals(String sql) { + return sql.replaceAll("'(?:[^']|'')*'", "''"); + } + /** * Checks for invalid characters or sequences * diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/DatabaseServiceMoreTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/DatabaseServiceMoreTest.java index 6bf8422ef7..27643054ab 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/DatabaseServiceMoreTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/DatabaseServiceMoreTest.java @@ -168,4 +168,61 @@ class DatabaseServiceMoreTest { assertThat(resolved.startsWith(tempDir)).isTrue(); } } + + @Nested + @DisplayName("SQL import validation") + class SqlValidation { + + private Path writeScript(String sql) throws IOException { + Path script = Files.createTempFile("import", ".sql"); + Files.writeString(script, sql); + return script; + } + + @Test + @DisplayName("rejects FILE_WRITE") + void rejectsFileWrite() throws IOException { + Path script = writeScript("CREATE TABLE X AS SELECT FILE_WRITE('data', '/tmp/out');"); + assertThatThrownBy(() -> databaseService.importDatabaseFromUI(script)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + @DisplayName("rejects FILE_READ in an INSERT") + void rejectsFileRead() throws IOException { + Path script = writeScript("INSERT INTO PUBLIC.X(D) VALUES (FILE_READ('/tmp/in'));"); + assertThatThrownBy(() -> databaseService.importDatabaseFromUI(script)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + @DisplayName("rejects CREATE ALIAS") + void rejectsCreateAlias() throws IOException { + Path script = + writeScript( + "CREATE ALIAS MYALIAS AS 'String run(String c) throws Exception" + + " { return c; }';"); + assertThatThrownBy(() -> databaseService.importDatabaseFromUI(script)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + @DisplayName("rejects RUNSCRIPT FROM") + void rejectsRunscript() throws IOException { + Path script = writeScript("RUNSCRIPT FROM '/tmp/other.sql';"); + assertThatThrownBy(() -> databaseService.importDatabaseFromUI(script)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + @DisplayName("allows a backup whose data mentions a keyword") + void allowsKeywordInsideStringData() throws IOException { + Path script = + writeScript( + "CREATE TABLE PUBLIC.NOTES(ID INT, BODY CHARACTER VARYING);" + + " INSERT INTO PUBLIC.NOTES(ID, BODY)" + + " VALUES(1, 'plain text with FILE_WRITE() inside');"); + assertThat(databaseService.importDatabaseFromUI(script)).isTrue(); + } + } }