From cfd6d1988604f33ea09a027a5590a76df1b09e6b Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:33:15 +0100 Subject: [PATCH] Fix CodeQL SSRF and path-traversal findings in form-detection model manager --- .../FormDetectionModelController.java | 8 +----- .../service/FormDetectionModelManager.java | 21 ++++++++++------ .../FormDetectionModelControllerTest.java | 2 +- .../FormDetectionModelManagerTest.java | 25 +++++-------------- .../public/locales/en-US/translation.toml | 2 +- .../AdminFormDetectionSection.tsx | 2 +- 6 files changed, 23 insertions(+), 37 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelController.java b/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelController.java index 3db73c1fc5..6c8b3f55aa 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelController.java @@ -52,7 +52,7 @@ public class FormDetectionModelController { return ResponseEntity.badRequest().body(s); } try { - manager.startInstall(request.getModelId(), request.getOnnxUrl(), request.getSha256()); + manager.startInstall(request.getModelId()); return ResponseEntity.accepted().body(manager.status()); } catch (IllegalStateException e) { ModelStatusResponse s = manager.status(); @@ -115,11 +115,5 @@ public class FormDetectionModelController { @Data public static class InstallRequest { private String modelId; - - /** Optional override download URL (admin self-hosted export); falls back to the catalog. */ - private String onnxUrl; - - /** Optional override SHA-256; falls back to the catalog. */ - private String sha256; } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java b/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java index dc0e3262e0..c166391768 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManager.java @@ -156,17 +156,18 @@ public class FormDetectionModelManager { * @throws IllegalArgumentException unknown/invalid model id or bad checksum format * @throws IllegalStateException no URL/checksum configured, or an install is already running */ - public synchronized void startInstall(String modelId, String overrideUrl, String overrideSha) { + public synchronized void startInstall(String modelId) { + if (!SAFE_ID.matcher(modelId).matches()) { + throw new IllegalArgumentException("Invalid model id: " + modelId); + } ModelCatalogEntry entry = catalog.getById(modelId) .orElseThrow( () -> new IllegalArgumentException("Unknown model id: " + modelId)); - if (!SAFE_ID.matcher(modelId).matches()) { - throw new IllegalArgumentException("Invalid model id: " + modelId); - } - String url = StringUtils.isNotBlank(overrideUrl) ? overrideUrl : entry.getOnnxUrl(); - String rawSha = StringUtils.isNotBlank(overrideSha) ? overrideSha : entry.getSha256(); - String sha = rawSha == null ? null : rawSha.toLowerCase(Locale.ROOT); + // URL + checksum come ONLY from the bundled catalog (trusted constants), never from the + // request, so an admin cannot point the download at an arbitrary host (avoids SSRF). + String url = entry.getOnnxUrl(); + String sha = entry.getSha256() == null ? null : entry.getSha256().toLowerCase(Locale.ROOT); if (StringUtils.isBlank(url) || StringUtils.isBlank(sha)) { throw new IllegalStateException( "Model '" + modelId + "' has no download URL/checksum configured yet"); @@ -339,7 +340,11 @@ public class FormDetectionModelManager { if (StringUtils.isBlank(id) || !SAFE_ID.matcher(id).matches()) { return; } - Path file = modelDir().resolve(id + ".onnx"); + Path base = modelDir().toAbsolutePath().normalize(); + Path file = base.resolve(id + ".onnx").normalize(); + if (!file.startsWith(base)) { + return; // path-traversal guard (SAFE_ID already blocks it; this also satisfies CodeQL) + } try { Files.deleteIfExists(file); } catch (IOException e) { diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelControllerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelControllerTest.java index 58f9ef855f..274771f4d4 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelControllerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/controller/FormDetectionModelControllerTest.java @@ -72,7 +72,7 @@ class FormDetectionModelControllerTest { Mockito.when(manager.status()).thenReturn(notInstalled()); Mockito.doThrow(new IllegalStateException("An install is already in progress")) .when(manager) - .startInstall(Mockito.eq("ffdnet-s"), Mockito.any(), Mockito.any()); + .startInstall(Mockito.eq("ffdnet-s")); mvc(manager) .perform( diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java index 5f67250724..1f14eb5b0e 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/formdetection/service/FormDetectionModelManagerTest.java @@ -100,7 +100,7 @@ class FormDetectionModelManagerTest { FormDetectionModelManager m = manager(dir, entry("http://127.0.0.1:" + port + "/model.onnx", modelSha), ep); - m.startInstall("test-model", null, null); + m.startInstall("test-model"); awaitState(m, "ready", 5000); Path onnx = dir.resolve("test-model.onnx"); @@ -117,7 +117,7 @@ class FormDetectionModelManagerTest { FormDetectionModelManager m = manager(dir, entry("http://127.0.0.1:" + port + "/model.onnx", "0".repeat(64)), ep); - m.startInstall("test-model", null, null); + m.startInstall("test-model"); awaitState(m, "failed", 5000); assertFalse(Files.exists(dir.resolve("test-model.onnx")), "no model on mismatch"); @@ -147,9 +147,9 @@ class FormDetectionModelManagerTest { entry("http://127.0.0.1:" + port + "/gated.onnx", modelSha), Mockito.mock(EndpointConfiguration.class)); - m.startInstall("test-model", null, null); // begins, blocks in handler + m.startInstall("test-model"); // begins, blocks in handler // installing flag is set synchronously before the worker thread spawns - assertThrows(IllegalStateException.class, () -> m.startInstall("test-model", null, null)); + assertThrows(IllegalStateException.class, () -> m.startInstall("test-model")); gate.countDown(); awaitState(m, "ready", 5000); } @@ -158,7 +158,7 @@ class FormDetectionModelManagerTest { void rejectsBlankUrl(@TempDir Path dir) { FormDetectionModelManager m = manager(dir, entry("", ""), Mockito.mock(EndpointConfiguration.class)); - assertThrows(IllegalStateException.class, () -> m.startInstall("test-model", null, null)); + assertThrows(IllegalStateException.class, () -> m.startInstall("test-model")); } @Test @@ -168,19 +168,6 @@ class FormDetectionModelManagerTest { dir, entry("http://127.0.0.1:" + port + "/model.onnx", modelSha), Mockito.mock(EndpointConfiguration.class)); - assertThrows(IllegalArgumentException.class, () -> m.startInstall("unknown", null, null)); - } - - @Test - void overrideUrlAndShaWork(@TempDir Path dir) throws Exception { - EndpointConfiguration ep = Mockito.mock(EndpointConfiguration.class); - // catalog entry has blank url/sha; admin supplies overrides at install time - FormDetectionModelManager m = manager(dir, entry("", ""), ep); - - m.startInstall("test-model", "http://127.0.0.1:" + port + "/model.onnx", modelSha); - awaitState(m, "ready", 5000); - - assertTrue(Files.exists(dir.resolve("test-model.onnx"))); - assertTrue(m.isReady()); + assertThrows(IllegalArgumentException.class, () -> m.startInstall("unknown")); } } diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 30fb37e474..b007f20c63 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -583,7 +583,7 @@ show = "Air-gapped / offline install instructions" step1 = "1. On a machine with internet, download the model file:" step2 = "2. Verify its SHA-256 checksum matches:" step3 = "3. Copy it onto the Stirling-PDF server into the model directory:" -step4 = "4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). is the configs volume (e.g. /configs in Docker). Alternatively, point the Install button at an internal mirror using an override URL + checksum." +step4 = "4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). is the configs volume (e.g. /configs in Docker). The Docker image also pre-bundles this model, so it is ready with no download." [admin.formDetection.engine] auto = "Auto" diff --git a/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx b/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx index 9387249001..a55770de42 100644 --- a/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx +++ b/frontend/editor/src/proprietary/components/shared/config/configSections/AdminFormDetectionSection.tsx @@ -297,7 +297,7 @@ export default function AdminFormDetectionSection() { {t( "admin.formDetection.airgap.step4", - "4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). is the configs volume (e.g. /configs in Docker). Alternatively, point the Install button at an internal mirror using an override URL + checksum.", + "4. Set formDetection.activeModelId to this model's id in settings.yml, then restart (an installed model is auto-detected on boot). is the configs volume (e.g. /configs in Docker). The Docker image also pre-bundles this model, so it is ready with no download.", )}