Skip to content

Commit

Permalink
[security] Fix command injection in packages
Browse files Browse the repository at this point in the history
  • Loading branch information
quentinguidee committed Sep 25, 2023
1 parent 9222227 commit 2c770d4
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 18 deletions.
27 changes: 14 additions & 13 deletions adapter/package_fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var (
)

type PackageFSAdapter struct {
pkgs map[string]types.Package
pkgs []types.Package
pkgsMutex *sync.RWMutex

dependenciesPath string
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -90,7 +90,7 @@ func (a *PackageFSAdapter) Reload() error {
return err
}

a.set(name, *pkg)
a.pkgs = append(a.pkgs, *pkg)
}

return nil
Expand All @@ -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
}
8 changes: 6 additions & 2 deletions adapter/package_fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
6 changes: 3 additions & 3 deletions services/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions types/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const (
)

type Package struct {
ID string `json:"-"`
Name string `json:"name"`
Description string `json:"description"`
Homepage string `json:"homepage"`
Expand Down

0 comments on commit 2c770d4

Please sign in to comment.