fix: validate path traversal

Author: dcshzj

package-lock.json

@@ -41,6 +41,7 @@
     "@octokit/rest": "^20.1.1",
     "@opengovsg/formsg-sdk": "^0.11.0",
     "@opengovsg/sgid-client": "^2.0.0",
+    "@opengovsg/starter-kitty-validators": "^1.2.11",
     "@slack/bolt": "^3.19.0",
     "auto-bind": "^4.0.0",
     "aws-lambda": "^1.0.7",

package.json

@@ -1,14 +1,14 @@
 import fs from "fs"
 import path from "path"
 
+import { createPathSchema } from "@opengovsg/starter-kitty-validators"
 import { err, errAsync, ok, okAsync, Result, ResultAsync } from "neverthrow"
 import {
   CleanOptions,
   GitError,
   SimpleGit,
   DefaultLogFields,
   LogResult,
-  ListLogLine,
 } from "simple-git"
 
 import logger from "@logger/logger"
@@ -366,6 +366,18 @@ export default class GitFileSystemService {
     const efsVolPath = isStaging
       ? EFS_VOL_PATH_STAGING
       : EFS_VOL_PATH_STAGING_LITE
+
+    // Validate that the filePath is a valid relative path to prevent directory
+    // traversal attacks
+    const repoBaseDirectory = `${efsVolPath}/${repoName}`
+    const pathSchema = createPathSchema({ basePath: repoBaseDirectory })
+    const parsedPathResult = pathSchema.safeParse(filePath)
+
+    if (!parsedPathResult.success) {
+      logger.error(`Invalid file path: ${filePath} for repo: ${repoName}`)
+      return errAsync(new BadRequestError("Invalid file path"))
+    }
+
     return ResultAsync.fromPromise(
       fs.promises.stat(`${efsVolPath}/${repoName}/${filePath}`),
       (error) => {
@@ -985,35 +997,39 @@ export default class GitFileSystemService {
     encoding: "utf-8" | "base64" = "utf-8"
   ): ResultAsync<GitFile, GitFileSystemError | NotFoundError> {
     const defaultEfsVolPath = EFS_VOL_PATH_STAGING
-    return ResultAsync.combine([
-      ResultAsync.fromPromise(
-        fs.promises.readFile(
-          `${defaultEfsVolPath}/${repoName}/${filePath}`,
-          encoding
-        ),
-        (error) => {
-          if (error instanceof Error && error.message.includes("ENOENT")) {
-            return new NotFoundError("File does not exist")
-          }
+    return this.getFilePathStats(repoName, filePath, true)
+      .andThen(() =>
+        ResultAsync.combine([
+          ResultAsync.fromPromise(
+            fs.promises.readFile(
+              `${defaultEfsVolPath}/${repoName}/${filePath}`,
+              encoding
+            ),
+            (error) => {
+              if (error instanceof Error && error.message.includes("ENOENT")) {
+                return new NotFoundError("File does not exist")
+              }
 
-          logger.error(`Error when reading ${filePath}: ${error}`)
+              logger.error(`Error when reading ${filePath}: ${error}`)
 
-          if (error instanceof Error) {
-            return new GitFileSystemError("Unable to read file")
-          }
+              if (error instanceof Error) {
+                return new GitFileSystemError("Unable to read file")
+              }
 
-          return new GitFileSystemError("An unknown error occurred")
+              return new GitFileSystemError("An unknown error occurred")
+            }
+          ),
+          this.getGitBlobHash(repoName, filePath, true),
+        ])
+      )
+      .map((contentAndHash) => {
+        const [content, sha] = contentAndHash
+        const result: GitFile = {
+          content,
+          sha,
         }
-      ),
-      this.getGitBlobHash(repoName, filePath, true),
-    ]).map((contentAndHash) => {
-      const [content, sha] = contentAndHash
-      const result: GitFile = {
-        content,
-        sha,
-      }
-      return result
-    })
+        return result
+      })
   }
 
   getFileExtension(fileName: string): Result<string, MediaTypeError> {

src/services/db/GitFileSystemService.ts

@@ -720,6 +720,16 @@ describe("GitFileSystemService", () => {
       expect(result._unsafeUnwrap().isDirectory()).toBeTrue()
     })
 
+    it("should return a BadRequestError if the path goes above the repo root", async () => {
+      const result = await GitFileSystemService.getFilePathStats(
+        "fake-repo",
+        "../some-file",
+        DEFAULT_BRANCH === "staging"
+      )
+
+      expect(result._unsafeUnwrapErr()).toBeInstanceOf(BadRequestError)
+    })
+
     it("should return a NotFoundError for a non-existent path", async () => {
       const result = await GitFileSystemService.getFilePathStats(
         "fake-repo",