Merge pull request #2200 from nordic-institute/code-scanning-39

fix: ensure file is in expected directory

src/common/common-admin-api/src/main/java/org/niis/xroad/restapi/common/backup/repository/BackupRepository.java

@@ -61,7 +61,7 @@
 @Repository
 @RequiredArgsConstructor
 public class BackupRepository {
-    private static final String CONFIGURATION_BACKUP_PATH = SystemProperties.getConfBackupPath();
+    private final String configurationBackupPath = SystemProperties.getConfBackupPath();
     // Set maximum number of levels of directories to visit, subdirectories are excluded
     private static final int DIR_MAX_DEPTH = 1;
 
@@ -73,10 +73,10 @@ public class BackupRepository {
      * @return list of backup files
      */
     public List<BackupFile> getBackupFiles() {
-        var backupPath = Paths.get(CONFIGURATION_BACKUP_PATH);
+        var backupPath = Paths.get(configurationBackupPath);
         if (!Files.exists(backupPath)) {
             log.warn("Backup directory [{}] does not exist.",
-                    CONFIGURATION_BACKUP_PATH);
+                    configurationBackupPath);
             return Collections.emptyList();
         }
 
@@ -89,7 +89,7 @@ public List<BackupFile> getBackupFiles() {
                     })
                     .collect(Collectors.toList());
         } catch (IOException ioe) {
-            log.error("can't read backup files from configuration path ({})", CONFIGURATION_BACKUP_PATH, ioe);
+            log.error("can't read backup files from configuration path ({})", configurationBackupPath, ioe);
             return Collections.emptyList();
         }
     }
@@ -171,7 +171,7 @@ public OffsetDateTime writeBackupFile(String filename, byte[] content) {
      * Return configuration backup path with a trailing slash
      */
     public String getConfigurationBackupPath() {
-        return CONFIGURATION_BACKUP_PATH + (CONFIGURATION_BACKUP_PATH.endsWith(File.separator) ? "" : File.separator);
+        return configurationBackupPath + (configurationBackupPath.endsWith(File.separator) ? "" : File.separator);
     }
 
     /**
@@ -181,7 +181,11 @@ public String getConfigurationBackupPath() {
      * @return path to the file
      */
     public Path getAbsoluteBackupFilePath(String filename) {
-        return Paths.get(CONFIGURATION_BACKUP_PATH).resolve(filename);
+        final var resolved = Paths.get(configurationBackupPath).resolve(filename);
+        if (!resolved.normalize().startsWith(Paths.get(configurationBackupPath))) {
+            throw new ServiceException(INVALID_FILENAME, filename);
+        }
+        return resolved;
     }
 
 

src/common/common-admin-api/src/test/java/org/niis/xroad/restapi/common/backup/repository/BackupRepositoryTest.java

@@ -0,0 +1,82 @@
+/*
+ * The MIT License
+ * Copyright (c) 2019- Nordic Institute for Interoperability Solutions (NIIS)
+ * Copyright (c) 2018 Estonian Information System Authority (RIA),
+ * Nordic Institute for Interoperability Solutions (NIIS), Population Register Centre (VRK)
+ * Copyright (c) 2015-2017 Estonian Information System Authority (RIA), Population Register Centre (VRK)
+ * <p>
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ * <p>
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * <p>
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.niis.xroad.restapi.common.backup.repository;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.niis.xroad.common.exception.ServiceException;
+import org.niis.xroad.restapi.common.backup.service.BackupValidator;
+
+import java.io.IOException;
+import java.nio.file.Path;
+
+import static ee.ria.xroad.common.SystemProperties.CONF_BACKUP_PATH;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+@ExtendWith(MockitoExtension.class)
+class BackupRepositoryTest {
+
+    private static final String BACKUP_FILE_NAME = "file.zip";
+
+    @Mock
+    private BackupValidator backupValidator;
+
+    @TempDir
+    Path backupDir;
+
+    private BackupRepository repository;
+
+    @BeforeEach
+    void beforeEach() {
+        System.setProperty(CONF_BACKUP_PATH, backupDir.toAbsolutePath().toString());
+        repository = new BackupRepository(backupValidator);
+    }
+
+    @Test
+    void getAbsoluteBackupFilePath() {
+        assertThat(repository.getAbsoluteBackupFilePath(BACKUP_FILE_NAME)).isEqualTo(backupDir.resolve(BACKUP_FILE_NAME));
+    }
+
+    @Test
+    void fileExists() throws IOException {
+        assertThat(repository.fileExists(BACKUP_FILE_NAME)).isFalse();
+        backupDir.resolve(BACKUP_FILE_NAME).toFile().createNewFile();
+        assertThat(repository.fileExists(BACKUP_FILE_NAME)).isTrue();
+    }
+
+    @Test
+    void fileExistsShouldFailOnRelativePathName() throws IOException {
+        assertThatThrownBy(() -> repository.fileExists("../secret/folder/file.txt"))
+                .isInstanceOf(ServiceException.class)
+                .hasMessage("Invalid filename");
+    }
+
+}