From c8763c2a604f6862604b12953455e5c92fb9814c Mon Sep 17 00:00:00 2001 From: Varun Prasad Date: Tue, 1 Sep 2026 00:19:48 +0530 Subject: [PATCH] BAH-5041 | Fix path traversal on v2 patientImage endpoint patientUuid was concatenated directly into the filesystem path in getPatientImageFileWithoutDefault() without normalization, allowing ../ traversal outside the configured images directory. Apply the same Path.normalize().startsWith() containment check already used for document deletion and saving. --- .../service/impl/PatientDocumentServiceImpl.java | 10 +++++++++- .../service/impl/PatientDocumentServiceImplTest.java | 11 +++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImpl.java b/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImpl.java index 211f81244..9f7de51c8 100644 --- a/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImpl.java +++ b/bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImpl.java @@ -209,6 +209,9 @@ public ResponseEntity retriveImage(String patientUuid) { @Override public ResponseEntity retriveImageWithoutDefault(String patientUuid) { File file = getPatientImageFileWithoutDefault(patientUuid); + if (file == null) { + return new ResponseEntity<>(HttpStatus.NOT_FOUND); + } return readImage(file); } @@ -261,7 +264,12 @@ private File getPatientImageFile(String patientUuid) { } private File getPatientImageFileWithoutDefault(String patientUuid) { - return new File(String.format("%s/%s.%s", BahmniCoreProperties.getProperty("bahmnicore.images.directory"), patientUuid, patientImagesFormat)); + Path base = Paths.get(BahmniCoreProperties.getProperty("bahmnicore.images.directory")).toAbsolutePath().normalize(); + Path resolved = base.resolve(patientUuid + "." + patientImagesFormat).normalize(); + if (!resolved.startsWith(base)) { + return null; + } + return resolved.toFile(); } private ResponseEntity readImage(File file) { diff --git a/bahmnicore-api/src/test/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImplTest.java b/bahmnicore-api/src/test/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImplTest.java index c1611333b..90f80ddff 100644 --- a/bahmnicore-api/src/test/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImplTest.java +++ b/bahmnicore-api/src/test/java/org/bahmni/module/bahmnicore/service/impl/PatientDocumentServiceImplTest.java @@ -89,6 +89,17 @@ public void shouldCreateRightDirectoryAccordingToPatientId() { absoluteFileDirectory.delete(); } + @Test + public void shouldReturn404WhenPathTraversalAttemptedViaPatientUuidOnV2() { + PowerMockito.mockStatic(BahmniCoreProperties.class); + when(BahmniCoreProperties.getProperty("bahmnicore.images.directory")).thenReturn("/bahmni_data/patient_images"); + patientDocumentService = new PatientDocumentServiceImpl(); + + ResponseEntity responseEntity = patientDocumentService.retriveImageWithoutDefault("../../../../tmp/secret"); + + assertEquals(404, responseEntity.getStatusCode().value()); + } + @Test public void shouldGetImageNotFoundForIfNoImageCapturedForPatientAndNoDefaultImageNotPresent() throws Exception { final FileInputStream fileInputStreamMock = PowerMockito.mock(FileInputStream.class);