Merge branch 'hotfix' into dev

adapter/package_fs.go

@@ -18,7 +18,7 @@ var (
 )
 
 type PackageFSAdapter struct {
-	pkgs      map[string]types.Package
+	pkgs      []types.Package
 	pkgsMutex *sync.RWMutex
 
 	dependenciesPath string
@@ -37,7 +37,7 @@ func NewPackageFSAdapter(params *PackageFSAdapterParams) types.PackageAdapterPor
 	}
 
 	adapter := &PackageFSAdapter{
-		pkgs:      map[string]types.Package{},
+		pkgs:      []types.Package{},
 		pkgsMutex: &sync.RWMutex{},
 
 		dependenciesPath: params.dependenciesPath,
@@ -54,18 +54,13 @@ func (a *PackageFSAdapter) GetByID(id string) (types.Package, error) {
 	a.pkgsMutex.RLock()
 	defer a.pkgsMutex.RUnlock()
 
-	pkg, ok := a.pkgs[id]
-	if !ok {
-		return types.Package{}, ErrPkgNotFound
+	for _, pkg := range a.pkgs {
+		if pkg.ID == id {
+			return pkg, nil
+		}
 	}
-	return pkg, nil
-}
 
-func (a *PackageFSAdapter) set(id string, pkg types.Package) {
-	a.pkgsMutex.Lock()
-	defer a.pkgsMutex.Unlock()
-
-	a.pkgs[id] = pkg
+	return types.Package{}, ErrPkgNotFound
 }
 
 func (a *PackageFSAdapter) GetPath(id string) string {
@@ -78,6 +73,11 @@ func (a *PackageFSAdapter) Reload() error {
 		return err
 	}
 
+	a.pkgsMutex.RLock()
+	defer a.pkgsMutex.RUnlock()
+
+	a.pkgs = []types.Package{}
+
 	for _, entry := range dir {
 		if !entry.IsDir() {
 			continue
@@ -90,7 +90,7 @@ func (a *PackageFSAdapter) Reload() error {
 			return err
 		}
 
-		a.set(name, *pkg)
+		a.pkgs = append(a.pkgs, *pkg)
 	}
 
 	return nil
@@ -106,5 +106,6 @@ func (a *PackageFSAdapter) readFromDisk(id string) (*types.Package, error) {
 
 	var pkg types.Package
 	err = json.Unmarshal(file, &pkg)
+	pkg.ID = id
 	return &pkg, err
 }

adapter/package_fs_test.go

@@ -29,8 +29,12 @@ func (suite *PackageAdapterTestSuite) SetupSuite() {
 	err := suite.adapter.Reload()
 	assert.NoError(suite.T(), err)
 	assert.NotEqual(suite.T(), 0, len(suite.adapter.pkgs))
-	assert.Equal(suite.T(), "Redis", suite.adapter.pkgs["redis"].Name)
-	assert.Equal(suite.T(), "BSD-3", suite.adapter.pkgs["redis"].License)
+
+	pkg, err := suite.adapter.GetByID("redis")
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), "redis", pkg.ID)
+	assert.Equal(suite.T(), "Redis", pkg.Name)
+	assert.Equal(suite.T(), "BSD-3", pkg.License)
 }
 
 func (suite *PackageAdapterTestSuite) TestGetPath() {

pkg/varchiver/tar.go

@@ -2,16 +2,86 @@ package varchiver
 
 import (
 	"archive/tar"
+	"archive/zip"
 	"compress/gzip"
+	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path"
+	"strings"
 )
 
+var (
+	ErrZipSlipAttack = errors.New("security: paths must be local")
+)
+
+func Unzip(src string, dest string) error {
+	if zipSlipAttack(src) || zipSlipAttack(dest) {
+		return ErrZipSlipAttack
+	}
+
+	reader, err := zip.OpenReader(src)
+	if err != nil {
+		return err
+	}
+
+	for _, header := range reader.File {
+		if zipSlipAttack(header.Name) {
+			return ErrZipSlipAttack
+		}
+
+		p := path.Join(dest, header.Name)
+
+		if zipSlipAttack(p) {
+			return ErrZipSlipAttack
+		}
+
+		if header.FileInfo().IsDir() {
+			err = os.MkdirAll(p, os.ModePerm)
+			if err != nil {
+				return err
+			}
+		} else {
+			err = os.MkdirAll(path.Dir(p), os.ModePerm)
+			if err != nil {
+				return err
+			}
+
+			file, err := os.Create(p)
+			if err != nil {
+				return err
+			}
+
+			content, err := header.Open()
+			if err != nil {
+				return err
+			}
+
+			_, err = io.Copy(file, content)
+			if err != nil {
+				return err
+			}
+
+			err = os.Chmod(p, 0755)
+			if err != nil {
+				return err
+			}
+
+			file.Close()
+		}
+	}
+
+	return nil
+}
+
 // Untar a tarball to a destination. src is the path to
 // the tarball, and dest is the path to the destination directory.
 func Untar(src string, dest string) error {
+	if zipSlipAttack(src) || zipSlipAttack(dest) {
+		return ErrZipSlipAttack
+	}
+
 	archive, err := os.Open(src)
 	if err != nil {
 		return err
@@ -36,21 +106,29 @@ func Untar(src string, dest string) error {
 			return err
 		}
 
-		filepath := path.Join(dest, header.Name)
+		if zipSlipAttack(header.Name) {
+			return ErrZipSlipAttack
+		}
+
+		p := path.Join(dest, header.Name)
+
+		if zipSlipAttack(p) {
+			return ErrZipSlipAttack
+		}
 
 		switch header.Typeflag {
 		case tar.TypeDir:
-			err = os.MkdirAll(filepath, os.ModePerm)
+			err = os.MkdirAll(p, os.ModePerm)
 			if err != nil {
 				return err
 			}
 		case tar.TypeReg:
-			err := os.MkdirAll(path.Dir(filepath), os.ModePerm)
+			err := os.MkdirAll(path.Dir(p), os.ModePerm)
 			if err != nil {
 				return err
 			}
 
-			file, err := os.Create(filepath)
+			file, err := os.Create(p)
 			if err != nil {
 				return err
 			}
@@ -60,7 +138,7 @@ func Untar(src string, dest string) error {
 				return err
 			}
 
-			err = os.Chmod(filepath, 0755)
+			err = os.Chmod(p, 0755)
 			if err != nil {
 				return err
 			}
@@ -73,3 +151,8 @@ func Untar(src string, dest string) error {
 
 	return nil
 }
+
+// CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
+func zipSlipAttack(path string) bool {
+	return strings.Contains(path, "..")
+}

services/dependencies.go

@@ -1,7 +1,6 @@
 package services
 
 import (
-	"archive/zip"
 	"context"
 	"errors"
 	"fmt"
@@ -17,6 +16,7 @@ import (
 	"github.com/vertex-center/vertex/config"
 	"github.com/vertex-center/vertex/pkg/log"
 	"github.com/vertex-center/vertex/pkg/storage"
+	"github.com/vertex-center/vertex/pkg/varchiver"
 	"github.com/vertex-center/vertex/pkg/vdocker"
 	"github.com/vertex-center/vertex/types"
 	"github.com/vertex-center/vlog"
@@ -245,12 +245,14 @@ func (d *clientUpdater) InstallUpdate() error {
 				return err
 			}
 
-			err = download(dir, *asset.BrowserDownloadURL)
+			tempZipPath := path.Join(dir, "temp.zip")
+
+			err = download(tempZipPath, *asset.BrowserDownloadURL)
 			if err != nil {
 				return err
 			}
 
-			err = unarchive(dir)
+			err = varchiver.Unzip(tempZipPath, dir)
 			if err != nil {
 				return err
 			}
@@ -293,7 +295,7 @@ func download(dir string, url string) error {
 	}
 	defer res.Body.Close()
 
-	file, err := os.Create(path.Join(dir, "temp.zip"))
+	file, err := os.Create(dir)
 	if err != nil {
 		return err
 	}
@@ -303,53 +305,6 @@ func download(dir string, url string) error {
 	return err
 }
 
-func unarchive(dir string) error {
-	reader, err := zip.OpenReader(path.Join(dir, "temp.zip"))
-	if err != nil {
-		return err
-	}
-
-	for _, header := range reader.File {
-		filepath := path.Join(dir, header.Name)
-
-		if header.FileInfo().IsDir() {
-			err = os.MkdirAll(filepath, os.ModePerm)
-			if err != nil {
-				return err
-			}
-		} else {
-			err = os.MkdirAll(path.Dir(filepath), os.ModePerm)
-			if err != nil {
-				return err
-			}
-
-			file, err := os.Create(filepath)
-			if err != nil {
-				return err
-			}
-
-			content, err := header.Open()
-			if err != nil {
-				return err
-			}
-
-			_, err = io.Copy(file, content)
-			if err != nil {
-				return err
-			}
-
-			err = os.Chmod(filepath, 0755)
-			if err != nil {
-				return err
-			}
-
-			file.Close()
-		}
-	}
-
-	return nil
-}
-
 type gitHubUpdater struct {
 	dir  string
 	name string

services/package.go

@@ -87,13 +87,13 @@ func (s *PackageService) GetByID(id string) (types.Package, error) {
 		return types.Package{}, err
 	}
 
-	installed, err := s.checkIsInstalled(id, pkg)
+	installed, err := s.checkIsInstalled(pkg)
 	pkg.Installed = &installed
 	return pkg, err
 }
 
-func (s *PackageService) checkIsInstalled(id string, pkg types.Package) (bool, error) {
-	pkgPath := s.packageAdapter.GetPath(id)
+func (s *PackageService) checkIsInstalled(pkg types.Package) (bool, error) {
+	pkgPath := s.packageAdapter.GetPath(pkg.ID)
 	isScript := strings.HasPrefix(pkg.Check, "script:")
 
 	if isScript {

types/package.go

@@ -10,6 +10,7 @@ const (
 )
 
 type Package struct {
+	ID             string            `json:"-"`
 	Name           string            `json:"name"`
 	Description    string            `json:"description"`
 	Homepage       string            `json:"homepage"`