From b40cf14df6da6102360b0d4307d5b75c97f3f77b Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Sat, 16 Sep 2023 03:31:52 +0700 Subject: [PATCH] fix(terraform): detect recursive modules (#1454) * fix(terraform): detect recursive modules * clean up src path --- pkg/scanners/terraform/parser/load_module.go | 17 ++----- .../terraform/parser/resolvers/local.go | 43 +++++++++++++--- .../terraform/parser/resolvers/local_test.go | 51 +++++++++++++++++++ .../recursive/child_module/foo/foo.tf | 3 ++ .../testdata/recursive/dot_source/foo.tf | 3 ++ 5 files changed, 98 insertions(+), 19 deletions(-) create mode 100644 pkg/scanners/terraform/parser/resolvers/local_test.go create mode 100644 pkg/scanners/terraform/parser/resolvers/testdata/recursive/child_module/foo/foo.tf create mode 100644 pkg/scanners/terraform/parser/resolvers/testdata/recursive/dot_source/foo.tf diff --git a/pkg/scanners/terraform/parser/load_module.go b/pkg/scanners/terraform/parser/load_module.go index dc3c5f19a..b6f1520b1 100644 --- a/pkg/scanners/terraform/parser/load_module.go +++ b/pkg/scanners/terraform/parser/load_module.go @@ -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 { @@ -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()) } @@ -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 } diff --git a/pkg/scanners/terraform/parser/resolvers/local.go b/pkg/scanners/terraform/parser/resolvers/local.go index 94d92099b..8d16cf3f4 100644 --- a/pkg/scanners/terraform/parser/resolvers/local.go +++ b/pkg/scanners/terraform/parser/resolvers/local.go @@ -2,7 +2,10 @@ package resolvers import ( "context" + "errors" + "fmt" "io/fs" + "os" "path/filepath" ) @@ -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 } diff --git a/pkg/scanners/terraform/parser/resolvers/local_test.go b/pkg/scanners/terraform/parser/resolvers/local_test.go new file mode 100644 index 000000000..103fd2d7a --- /dev/null +++ b/pkg/scanners/terraform/parser/resolvers/local_test.go @@ -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") + }) + } +} diff --git a/pkg/scanners/terraform/parser/resolvers/testdata/recursive/child_module/foo/foo.tf b/pkg/scanners/terraform/parser/resolvers/testdata/recursive/child_module/foo/foo.tf new file mode 100644 index 000000000..8a103e423 --- /dev/null +++ b/pkg/scanners/terraform/parser/resolvers/testdata/recursive/child_module/foo/foo.tf @@ -0,0 +1,3 @@ +module "foo" { + source = "../foo" +} \ No newline at end of file diff --git a/pkg/scanners/terraform/parser/resolvers/testdata/recursive/dot_source/foo.tf b/pkg/scanners/terraform/parser/resolvers/testdata/recursive/dot_source/foo.tf new file mode 100644 index 000000000..3069a8497 --- /dev/null +++ b/pkg/scanners/terraform/parser/resolvers/testdata/recursive/dot_source/foo.tf @@ -0,0 +1,3 @@ +module "foo" { + source = "./." +} \ No newline at end of file