From b60ddc21badce3a625593899d5428de5fcee02a5 Mon Sep 17 00:00:00 2001
From: ggkitsas <ggkitsas@yahoo.com>
Date: Sun, 2 Aug 2020 16:09:02 +0100
Subject: [PATCH] feat: adds support for path.Join and for tar archives in G305

---
 README.md           |  2 +-
 rules/archive.go    | 23 +++++++++------
 testutils/source.go | 72 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 86 insertions(+), 11 deletions(-)

diff --git a/README.md b/README.md
index 8fd10f1..5586d33 100644
--- a/README.md
+++ b/README.md
@@ -107,7 +107,7 @@ directory you can supply `./...` as the input argument.
 - G302: Poor file permissions used with chmod
 - G303: Creating tempfile using a predictable path
 - G304: File path provided as taint input
-- G305: File traversal when extracting zip archive
+- G305: File traversal when extracting zip/tar archive
 - G306: Poor file permissions used when writing to a new file
 - G307: Deferring a method which returns an error
 - G401: Detect the usage of DES, RC4, MD5 or SHA1
diff --git a/rules/archive.go b/rules/archive.go
index ca7a46e..92c7e44 100644
--- a/rules/archive.go
+++ b/rules/archive.go
@@ -9,15 +9,15 @@ import (
 
 type archive struct {
 	gosec.MetaData
-	calls   gosec.CallList
-	argType string
+	calls    gosec.CallList
+	argTypes []string
 }
 
 func (a *archive) ID() string {
 	return a.MetaData.ID
 }
 
-// Match inspects AST nodes to determine if the filepath.Joins uses any argument derived from type zip.File
+// Match inspects AST nodes to determine if the filepath.Joins uses any argument derived from type zip.File or tar.Header
 func (a *archive) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
 	if node := a.calls.ContainsPkgCallExpr(n, c, false); node != nil {
 		for _, arg := range node.Args {
@@ -35,26 +35,31 @@ func (a *archive) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
 				}
 			}
 
-			if argType != nil && argType.String() == a.argType {
-				return gosec.NewIssue(c, n, a.ID(), a.What, a.Severity, a.Confidence), nil
+			if argType != nil {
+				for _, t := range a.argTypes {
+					if argType.String() == t {
+						return gosec.NewIssue(c, n, a.ID(), a.What, a.Severity, a.Confidence), nil
+					}
+				}
 			}
 		}
 	}
 	return nil, nil
 }
 
-// NewArchive creates a new rule which detects the file traversal when extracting zip archives
+// NewArchive creates a new rule which detects the file traversal when extracting zip/tar archives
 func NewArchive(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 	calls := gosec.NewCallList()
 	calls.Add("path/filepath", "Join")
+	calls.Add("path", "Join")
 	return &archive{
-		calls:   calls,
-		argType: "*archive/zip.File",
+		calls:    calls,
+		argTypes: []string{"*archive/zip.File", "*archive/tar.Header"},
 		MetaData: gosec.MetaData{
 			ID:         id,
 			Severity:   gosec.Medium,
 			Confidence: gosec.High,
-			What:       "File traversal when extracting zip archive",
+			What:       "File traversal when extracting zip/tar archive",
 		},
 	}, []ast.Node{(*ast.CallExpr)(nil)}
 }
diff --git a/testutils/source.go b/testutils/source.go
index ee69766..7b17094 100644
--- a/testutils/source.go
+++ b/testutils/source.go
@@ -1588,7 +1588,7 @@ func main() {
 	log.Print(body)
 }`}, 1, gosec.NewConfig()}}
 
-	// SampleCodeG305 - File path traversal when extracting zip archives
+	// SampleCodeG305 - File path traversal when extracting zip/tar archives
 	SampleCodeG305 = []CodeSample{{[]string{`
 package unzip
 
@@ -1680,6 +1680,76 @@ func unzip(archive, target string) error {
 	}
 
 	return nil
+}`}, 1, gosec.NewConfig()}, {[]string{`
+package zip
+
+import (
+    "archive/zip"
+    "io"
+    "os"
+    "path"
+)
+
+func extractFile(f *zip.File, destPath string) error {
+    filePath := path.Join(destPath, f.Name)
+    os.MkdirAll(path.Dir(filePath), os.ModePerm)
+
+    rc, err := f.Open()
+    if err != nil {
+        return err
+    }
+    defer rc.Close()
+
+    fw, err := os.Create(filePath)
+    if err != nil {
+        return err
+    }
+    defer fw.Close()
+
+    if _, err = io.Copy(fw, rc); err != nil {
+        return err
+    }
+
+    if f.FileInfo().Mode()&os.ModeSymlink != 0 {
+        return nil
+    }
+
+    if err = os.Chtimes(filePath, f.ModTime(), f.ModTime()); err != nil {
+        return err
+    }
+    return os.Chmod(filePath, f.FileInfo().Mode())
+}`}, 1, gosec.NewConfig()}, {[]string{`
+package tz
+
+import (
+    "archive/tar"
+    "io"
+    "os"
+    "path"
+)
+
+func extractFile(f *tar.Header, tr *tar.Reader, destPath string) error {
+    filePath := path.Join(destPath, f.Name)
+    os.MkdirAll(path.Dir(filePath), os.ModePerm)
+
+    fw, err := os.Create(filePath)
+    if err != nil {
+        return err
+    }
+    defer fw.Close()
+
+    if _, err = io.Copy(fw, tr); err != nil {
+        return err
+    }
+
+    if f.FileInfo().Mode()&os.ModeSymlink != 0 {
+        return nil
+    }
+
+    if err = os.Chtimes(filePath, f.FileInfo().ModTime(), f.FileInfo().ModTime()); err != nil {
+        return err
+    }
+    return os.Chmod(filePath, f.FileInfo().Mode())
 }`}, 1, gosec.NewConfig()}}
 
 	// SampleCodeG306 - Poor permissions for WriteFile