Go: New File System Access Sinks

go/ql/lib/semmle/go/security/FileSystemAccess.qll

@@ -1,211 +1,217 @@
 import go
 
 /**
- * Provide File system access sinks of [fasthttp](https://github.com/valyala/fasthttp) web framework
+ * The File system access sinks of [fasthttp](https://github.com/valyala/fasthttp) web framework
  */
 class FastHttpFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   FastHttpFileSystemAccess() {
-    exists(DataFlow::Method mcn |
+    exists(Method m |
       (
-        mcn.hasQualifiedName("github.com/valyala/fasthttp.RequestCtx", ["SendFileBytes", "SendFile"]) or
-        mcn.hasQualifiedName("github.com/valyala/fasthttp.Response", ["SendFile"])
+        m.hasQualifiedName(package("github.com/valyala/fasthttp", ""), "RequestCtx",
+          ["SendFileBytes", "SendFile"])
+        or
+        m.hasQualifiedName(package("github.com/valyala/fasthttp", ""), "Response", "SendFile")
       ) and
-      this = mcn.getACall()
+      this = m.getACall() and
+      pathArg = 0
     )
     or
-    exists(DataFlow::Function f |
-      f.hasQualifiedName("github.com/valyala/fasthttp",
+    exists(Function f |
+      f.hasQualifiedName(package("github.com/valyala/fasthttp", ""),
         [
           "ServeFile", "ServeFileUncompressed", "ServeFileBytes", "ServeFileBytesUncompressed",
           "SaveMultipartFile"
         ]) and
-      this = f.getACall()
+      this = f.getACall() and
+      pathArg = 1
     )
   }
 
-  override DataFlow::Node getAPathArgument() {
-    this.getTarget().getName() =
-      [
-        "ServeFile", "ServeFileUncompressed", "ServeFileBytes", "ServeFileBytesUncompressed",
-        "SaveMultipartFile"
-      ] and
-    result = this.getArgument(1)
-    or
-    this.getTarget().getName() = ["SendFile", "SendFileBytes"] and
-    result = this.getArgument(0)
-  }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
 }
 
 /**
- * Provide File system access sinks of `net/http` package
+ * The File system access sinks of `net/http` package
  */
 class HttpServeFile extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   HttpServeFile() {
-    exists(DataFlow::Function mcn |
-      mcn.hasQualifiedName("net/http", "ServeFile") and
-      this = mcn.getACall()
+    exists(Function f |
+      f.hasQualifiedName("net/http", "ServeFile") and
+      this = f.getACall() and
+      pathArg = 2
     )
   }
 
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(2) }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
 }
 
 /**
- * Provide File system access sinks of [beego](https://github.com/beego/beego) web framework
+ * The File system access sinks of [beego](https://github.com/beego/beego) web framework
  */
 class BeegoFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   BeegoFileSystemAccess() {
-    exists(DataFlow::Method mcn |
+    exists(Method m |
       (
-        mcn.hasQualifiedName("github.com/beego/beego/v2/server/web/context.BeegoOutput", "Download") or
-        mcn.hasQualifiedName("github.com/beego/beego/v2/server/web.Controller",
-          "SaveToFileWithBuffer")
+        m.hasQualifiedName(package("github.com/beego/beego", "server/web/context"), "BeegoOutput",
+          "Download") and
+        pathArg = 0
+        or
+        m.hasQualifiedName(package("github.com/beego/beego", "server/web"), "Controller",
+          "SaveToFileWithBuffer") and
+        pathArg = 1
       ) and
-      this = mcn.getACall()
+      this = m.getACall()
     )
   }
 
-  override DataFlow::Node getAPathArgument() {
-    this.getTarget()
-        .hasQualifiedName("github.com/beego/beego/v2/server/web/context.BeegoOutput", "Download") and
-    result = this.getArgument(0)
-    or
-    this.getTarget()
-        .hasQualifiedName("github.com/beego/beego/v2/server/web.Controller", "SaveToFileWithBuffer") and
-    result = this.getArgument(1)
-  }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
 }
 
 /**
  * Provide File system access sinks of [beego](https://github.com/beego/beego) web framework
  */
 class EchoFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   EchoFileSystemAccess() {
-    exists(DataFlow::Method mcn |
-      mcn.hasQualifiedName("github.com/labstack/echo/v4.Context", ["Attachment", "File"]) and
-      this = mcn.getACall()
+    exists(Method m |
+      m.hasQualifiedName(package("github.com/labstack/echo", ""), "Context", ["Attachment", "File"]) and
+      this = m.getACall() and
+      pathArg = 0
     )
   }
 
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
 }
 
 /**
- * Provide File system access sinks of [gin](https://github.com/gin-gonic/gin) web framework
+ * The File system access sinks of [gin](https://github.com/gin-gonic/gin) web framework
  */
 class GinFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   GinFileSystemAccess() {
-    exists(DataFlow::Method mcn |
-      mcn.hasQualifiedName("github.com/gin-gonic/gin.Context",
-        ["File", "FileAttachment", "SaveUploadedFile"]) and
-      this = mcn.getACall()
+    exists(Method m |
+      (
+        m.hasQualifiedName(package("github.com/gin-gonic/gin", ""), "Context",
+          ["File", "FileAttachment"]) and
+        pathArg = 0
+        or
+        m.hasQualifiedName(package("github.com/gin-gonic/gin", ""), "Context", "SaveUploadedFile") and
+        pathArg = 1
+      ) and
+      this = m.getACall()
     )
   }
 
-  override DataFlow::Node getAPathArgument() {
-    this.getTarget().getName() = ["File", "FileAttachment"] and result = this.getArgument(0)
-    or
-    this.getTarget().getName() = "SaveUploadedFile" and result = this.getArgument(1)
-  }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
 }
 
 /**
- * Provide File system access sinks of [iris](https://github.com/kataras/iris) web framework
+ * The File system access sinks of [iris](https://github.com/kataras/iris) web framework
  */
 class IrisFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   IrisFileSystemAccess() {
-    exists(DataFlow::Method mcn |
-      mcn.hasQualifiedName("github.com/kataras/iris/v12/context.Context",
-        ["SendFile", "ServeFile", "SendFileWithRate", "ServeFileWithRate", "UploadFormFiles"]) and
-      this = mcn.getACall()
-      or
-      mcn.hasQualifiedName("github.com/kataras/iris/v12/context.Context", "SaveFormFile") and
-      this = mcn.getACall()
+    exists(Method m |
+      (
+        m.hasQualifiedName(package("github.com/kataras/iris", "context"), "Context",
+          ["SendFile", "ServeFile", "SendFileWithRate", "ServeFileWithRate", "UploadFormFiles"]) and
+        pathArg = 0
+        or
+        m.hasQualifiedName(package("github.com/kataras/iris", "context"), "Context", "SaveFormFile") and
+        pathArg = 1
+      ) and
+      this = m.getACall()
     )
   }
 
-  override DataFlow::Node getAPathArgument() {
-    this.getTarget().getName() =
-      ["SendFile", "ServeFile", "SendFileWithRate", "ServeFileWithRate", "UploadFormFiles"] and
-    result = this.getArgument(0)
-    or
-    this.getTarget().getName() = "SaveFormFile" and result = this.getArgument(1)
-  }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
 }
 
 /**
- * Provide File system access sinks of [fiber](https://github.com/gofiber/fiber) web framework
+ * The File system access sinks of [fiber](https://github.com/gofiber/fiber) web framework
  */
 class FiberSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   FiberSystemAccess() {
-    exists(DataFlow::Method mcn |
-      mcn.hasQualifiedName("github.com/gofiber/fiber/v2.Ctx", ["Attachment", "SendFile"]) and
-      this = mcn.getACall()
-      or
-      mcn.hasQualifiedName("github.com/gofiber/fiber/v2.Ctx", "SaveFile") and
-      this = mcn.getACall()
+    exists(Method m |
+      (
+        m.hasQualifiedName(package("github.com/gofiber/fiber", ""), "Ctx", "SendFile") and
+        pathArg = 0
+        or
+        m.hasQualifiedName(package("github.com/gofiber/fiber", ""), "Ctx", "SaveFile") and
+        pathArg = 1
+      ) and
+      this = m.getACall()
     )
   }
 
-  override DataFlow::Node getAPathArgument() {
-    this.getTarget().getName() = ["Attachment", "SendFile"] and result = this.getArgument(0)
-    or
-    this.getTarget().getName() = "SaveFile" and result = this.getArgument(1)
-  }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
+}
+
+predicate test(Function f) {
+      f.hasQualifiedName("github.com/valyala/fasthttp",
+        ["WriteReader", "SafeWriteReader", "WriteFile", "ReadFile", "ReadDir"]) 
+
 }
+string aferoPackage() { result = "github.com/valyala/fasthttp" }
 
 /**
  * Provide File system access sinks of [afero](https://github.com/spf13/afero) filesystem framework
- * The Only Type that is not vulnerable to path traversal is `afero.IOFS`
+ * The Types that are not vulnerable: `afero.BasePathFs` and `afero.IOFS`
  */
 class AferoSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
+  int pathArg;
+
   AferoSystemAccess() {
-    exists(DataFlow::Function mcn |
-      mcn.hasQualifiedName("github.com/spf13/afero",
+    // utility functions
+    exists(Function f |
+      f.hasQualifiedName("github.com/valyala/fasthttp",
         ["WriteReader", "SafeWriteReader", "WriteFile", "ReadFile", "ReadDir"]) and
-      this = mcn.getACall()
+      this = f.getACall() and
+      pathArg = 1
     )
     or
-    exists(DataFlow::Method mcn |
-      mcn.hasQualifiedName("github.com/spf13/afero.Afero",
-        ["ReadFile", "ReadDir", "WriteReader", "WriteFile", "SafeWriteReader"]) and
-      this = mcn.getACall()
-      or
-      mcn.hasQualifiedName("github.com/spf13/afero.HttpFs", ["Open", "OpenFile", "Create"]) and
-      this = mcn.getACall()
-      or
-      mcn.hasQualifiedName("github.com/spf13/afero.RegexpFs",
-        ["Create", "Open", "Remove", "OpenFile"]) and
-      this = mcn.getACall()
-      or
-      mcn.hasQualifiedName("github.com/spf13/afero.ReadOnlyFs",
-        ["Create", "Open", "Remove", "OpenFile", "ReadDir", "ReadlinkIfPossible"]) and
-      this = mcn.getACall()
+    // afero FS Types
+    exists(Method f |
+      f.hasQualifiedName(package(aferoPackage(), ""), "HttpFs",
+        ["Create", "Open", "OpenFile", "Remove", "RemoveAll"]) and
+      this = f.getACall() and
+      pathArg = 0
       or
-      mcn.hasQualifiedName("github.com/spf13/afero.OsFs",
-        ["Create", "Open", "Remove", "RemoveAll", "OpenFile", "ReadDir", "ReadlinkIfPossible"]) and
-      this = mcn.getACall()
+      f.hasQualifiedName(package(aferoPackage(), ""), "RegexpFs",
+        ["Create", "Open", "OpenFile", "Remove", "RemoveAll", "Mkdir", "MkdirAll"]) and
+      this = f.getACall() and
+      pathArg = 0
       or
-      mcn.hasQualifiedName("github.com/spf13/afero.OsFs",
-        ["Create", "Open", "Remove", "RemoveAll", "OpenFile", "ReadDir", "ReadlinkIfPossible"]) and
-      this = mcn.getACall()
+      f.hasQualifiedName(package(aferoPackage(), ""), "ReadOnlyFs",
+        ["Create", "Open", "OpenFile", "ReadDir", "ReadlinkIfPossible", "Mkdir", "MkdirAll"]) and
+      this = f.getACall() and
+      pathArg = 0
       or
-      mcn.hasQualifiedName("github.com/spf13/afero.MemMapFs",
-        ["Create", "Open", "OpenFile", "Remove", "RemoveAll"]) and
-      this = mcn.getACall()
+      f.hasQualifiedName(package(aferoPackage(), ""), "OsFs",
+        [
+          "Create", "Open", "OpenFile", "ReadlinkIfPossible", "Remove", "RemoveAll", "Mkdir",
+          "MkdirAll"
+        ]) and
+      this = f.getACall() and
+      pathArg = 0
       or
-      mcn.hasQualifiedName("github.com/spf13/afero.BasePathFs",
-        ["Create", "Open", "OpenFile", "Remove", "RemoveAll", "ReadlinkIfPossible"]) and
-      this = mcn.getACall()
+      f.hasQualifiedName(package(aferoPackage(), ""), "MemMapFs",
+        ["Create", "Open", "OpenFile", "Remove", "RemoveAll", "Mkdir", "MkdirAll"]) and
+      this = f.getACall() and
+      pathArg = 0
     )
   }
 
-  override DataFlow::Node getAPathArgument() {
-    if
-      this.getTarget()
-          .hasQualifiedName("github.com/spf13/afero",
-            ["WriteReader", "SafeWriteReader", "WriteFile", "ReadFile", "ReadDir"])
-    then result = this.getArgument(1)
-    else result = this.getArgument(0)
-  }
+  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArg) }
 }

go/ql/test/library-tests/semmle/go/security/FileSystemAccess/main.go

@@ -95,15 +95,10 @@ func Echo() {
 
 func Fiber() {
 	app := fiber.New()
-	app.Get("/a", func(c *fiber.Ctx) error {
-		filepath := c.Params("filepath")
-		return c.SendFile(filepath)
-	})
 	app.Get("/b", func(c *fiber.Ctx) error {
 		filepath := c.Params("filepath")
 		header, _ := c.FormFile("f")
 		_ = c.SaveFile(header, filepath)
-		c.Attachment(filepath)
 		return c.SendFile(filepath)
 	})
 	_ = app.Listen(":3000")