Skip to content

Commit

Permalink
fix G305: File traversal when extracting zip archive (gosec)
Browse files Browse the repository at this point in the history
  • Loading branch information
wongoo committed Sep 26, 2019
1 parent e801e1c commit 2100f25
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 29 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# `gracego` enables gracefully restart or upgrade golang application.
# `gracego` enables gracefully restart, upgrade or replace golang application.

## Usage

Expand Down
2 changes: 1 addition & 1 deletion borrow.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

func getBorrowSockFile(pid int) string {
return filepath.Join(os.TempDir(), fmt.Sprintf("grace_borrow_request_%d.sock", pid))
return filepath.Join(os.TempDir(), fmt.Sprintf("gracego_borrow_%d.sock", pid))
}

func borrow(addr string) (*os.File, error) {
Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
module github.com/vogo/gracego

go 1.12
require github.com/stretchr/testify v1.4.0

go 1.13
11 changes: 11 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
1 change: 1 addition & 0 deletions gracego.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2019 The vogo Authors. All rights reserved.
// author: wongoo

package gracego

Expand Down
1 change: 1 addition & 0 deletions log.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2019 The vogo Authors. All rights reserved.
// author: wongoo

package gracego

Expand Down
1 change: 1 addition & 0 deletions pid.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2019 The vogo Authors. All rights reserved.
// author: wongoo

package gracego

Expand Down
1 change: 1 addition & 0 deletions upgrade.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Copyright 2019 The vogo Authors. All rights reserved.
// author: wongoo

package gracego

Expand Down
56 changes: 30 additions & 26 deletions upzip.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Copyright 2019 The vogo Authors. All rights reserved.
// author: wongoo

package gracego

import (
"archive/zip"
"fmt"
"io"
"os"
"path/filepath"
Expand All @@ -25,42 +25,46 @@ func unzip(src, dest string) error {
return err
}

for _, f := range r.File {
// Store filename/path for returning and using later on
filePath := filepath.Join(dest, f.Name)

for _, zipFile := range r.File {
// Check for ZipSlip
if !strings.HasPrefix(filePath, filepath.Clean(dest)+string(os.PathSeparator)) {
return fmt.Errorf("%s: illegal file path", filePath)
}
fileName := strings.ReplaceAll(zipFile.Name, "..", "")

if f.FileInfo().IsDir() {
// Make Folder
if err := os.MkdirAll(filePath, os.ModePerm); err != nil {
return err
}
if fileName != zipFile.Name {
graceLog("ignore zip file: %s", zipFile.Name)
continue
}

outFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
if err != nil {
targetPath := filepath.Join(dest, fileName)

if err := writeZipFile(targetPath, zipFile); err != nil {
return err
}
}
return nil
}

rc, err := f.Open()
if err != nil {
func writeZipFile(targetPath string, f *zip.File) error {
if f.FileInfo().IsDir() {
if err := os.MkdirAll(targetPath, os.ModePerm); err != nil {
return err
}
return nil
}

_, err = io.Copy(outFile, rc)

// Close the file without defer to close before next iteration of loop
outFile.Close()
_ = rc.Close()
outFile, err := os.OpenFile(targetPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
if err != nil {
return err
}

if err != nil {
return err
}
rc, err := f.Open()
if err != nil {
return err
}
return nil

_, err = io.Copy(outFile, rc)

outFile.Close()
_ = rc.Close()

return err
}
29 changes: 29 additions & 0 deletions util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2019 The vogo Authors. All rights reserved.
// author: wongoo

package gracego

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
)

func TestFilePath(t *testing.T) {
assert.Equal(t, ".", filepath.Dir("test"))
assert.Equal(t, ".", filepath.Dir("."))
assert.Equal(t, ".", filepath.Dir("./"))
assert.Equal(t, ".", filepath.Dir("./test"))
assert.Equal(t, "..", filepath.Dir("./../test"))
assert.Equal(t, "../..", filepath.Dir("./../../test"))

assert.Equal(t, "/a/b/test", filepath.Clean("/a/b/test"))
assert.Equal(t, "/a/b/test", filepath.Clean("/a/b/test/"))
assert.Equal(t, "/test", filepath.Clean("/a/b/../../test"))

assert.Equal(t, "test", filepath.Clean("test"))
assert.Equal(t, "test", filepath.Clean("./test"))
assert.Equal(t, "../test", filepath.Clean("./../test"))
assert.Equal(t, "../../test", filepath.Clean("./../../test"))
}

0 comments on commit 2100f25

Please sign in to comment.