Skip to content

Commit

Permalink
fix(terraform): detect recursive modules (#1454)
Browse files Browse the repository at this point in the history
* fix(terraform): detect recursive modules

* clean up src path
  • Loading branch information
nikpivkin authored Sep 15, 2023
1 parent 65798c7 commit b40cf14
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 19 deletions.
17 changes: 5 additions & 12 deletions pkg/scanners/terraform/parser/load_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (

"github.com/aquasecurity/defsec/pkg/scanners/terraform/parser/resolvers"
"github.com/aquasecurity/defsec/pkg/terraform"

"github.com/zclconf/go-cty/cty"
)

type moduleLoadError struct {
Expand Down Expand Up @@ -82,16 +80,8 @@ func (e *evaluator) loadModule(ctx context.Context, b *terraform.Block) (*Module
return nil, fmt.Errorf("module without label at %s", metadata.Range())
}

var source string
attrs := b.Attributes()
for _, attr := range attrs {
if attr.Name() == "source" {
sourceVal := attr.Value()
if sourceVal.Type() == cty.String {
source = sourceVal.AsString()
}
}
}
source := b.GetAttribute("source").AsStringValueOrDefault("", b).Value()

if source == "" {
return nil, fmt.Errorf("could not read module source attribute at %s", metadata.Range().String())
}
Expand All @@ -112,6 +102,9 @@ func (e *evaluator) loadModuleFromTerraformCache(ctx context.Context, b *terrafo
name := b.ModuleName()
for _, module := range e.moduleMetadata.Modules {
if module.Key == name {
if filepath.Clean(module.Source) == module.Dir {
return nil, fmt.Errorf("module %q cannot use itself as a child", name)
}
modulePath = filepath.Clean(filepath.Join(e.projectRootPath, module.Dir))
break
}
Expand Down
43 changes: 36 additions & 7 deletions pkg/scanners/terraform/parser/resolvers/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package resolvers

import (
"context"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
)

Expand All @@ -14,13 +17,39 @@ func (r *localResolver) Resolve(_ context.Context, target fs.FS, opt Options) (f
if !opt.hasPrefix(".", "..") {
return nil, "", "", false, nil
}
joined := filepath.Clean(filepath.Join(opt.ModulePath, opt.Source))
if _, err := fs.Stat(target, filepath.ToSlash(joined)); err == nil {
opt.Debug("Module '%s' resolved locally to %s", opt.Name, joined)
return target, "", joined, true, nil

srcFullPath := filepath.Clean(filepath.Join(opt.ModulePath, opt.Source))

if same, err := sameModule(
target,
filepath.ToSlash(filepath.Clean(opt.ModulePath)),
filepath.ToSlash(srcFullPath),
); err != nil {
return nil, "", "", false, err
} else if same {
return nil, "", "", false, fmt.Errorf("module %q cannot use itself as a child", opt.Name)
}

opt.Debug("Module '%s' resolved locally to %s", opt.Name, srcFullPath)
return target, "", srcFullPath, true, nil
}

func sameModule(fsys fs.FS, module, childModule string) (bool, error) {
fi1, err := fs.Stat(fsys, module)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return false, nil
}
return false, fmt.Errorf("file stat error: %w", err)
}

fi2, err := fs.Stat(fsys, childModule)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return false, fmt.Errorf("module %q not found", childModule)
}
return false, fmt.Errorf("file stat error: %w", err)
}

clean := filepath.Clean(opt.Source)
opt.Debug("Module '%s' resolved locally to %s", opt.Name, clean)
return target, "", clean, true, nil
return os.SameFile(fi1, fi2), nil
}
51 changes: 51 additions & 0 deletions pkg/scanners/terraform/parser/resolvers/local_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package resolvers

import (
"context"
"os"
"testing"

"github.com/aquasecurity/defsec/pkg/debug"
"github.com/stretchr/testify/assert"
)

func Test_ResolveRecursiveSource(t *testing.T) {

tests := []struct {
name string
dir string
opt Options
}{
{
name: "child module",
dir: "testdata/recursive/child_module",
opt: Options{
Source: "../foo",
OriginalSource: "../foo",
WorkingDir: "foo",
Name: "module.foo",
ModulePath: "foo",
DebugLogger: debug.Logger{},
},
},
{
name: "dot source",
dir: "testdata/recursive/dot_source",
opt: Options{
Source: "./.",
OriginalSource: "./.",
WorkingDir: ".",
Name: "module.foo",
ModulePath: ".",
DebugLogger: debug.Logger{},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, _, _, _, err := Local.Resolve(context.TODO(), os.DirFS(tt.dir), tt.opt)
assert.ErrorContains(t, err, "cannot use itself as a child")
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module "foo" {
source = "../foo"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module "foo" {
source = "./."
}

0 comments on commit b40cf14

Please sign in to comment.