search

aquasecurity / trivy

Find vulnerabilities, misconfigurations, secrets, SBOM in containers, Kubernetes, code repositories, clouds and more
https://aquasecurity.github.io/trivy
Apache License 2.0
23.04k stars 2.27k forks source link

fix(terraform): panic when scanning a synthesized TF config using `cdktf` #5080

Closed nikpivkin closed 1 year ago

nikpivkin commented 1 year ago

Source:

Steps to reproduce:

  1. cdktf init --template=typescript --local
  2. npm install @cdktf/provider-aws
  3. Replace the contents of main.ts with:
import { Construct } from "constructs";
import { App, TerraformStack, TerraformOutput } from "cdktf";
import { AwsProvider } from "@cdktf/provider-aws/lib/provider";
import { KmsAlias } from "@cdktf/provider-aws/lib/kms-alias";
import { KmsKey } from "@cdktf/provider-aws/lib/kms-key";
import { S3Bucket } from "@cdktf/provider-aws/lib/s3-bucket";
import { S3BucketPublicAccessBlock } from "@cdktf/provider-aws/lib/s3-bucket-public-access-block"
import { DynamodbTable } from "@cdktf/provider-aws/lib/dynamodb-table";

export class tfBackend extends Construct {
  constructor(scope: Construct, name: string) {
    super(scope, name);

    // Create KMS Key
    const terraformKey = new KmsKey(this, "TerraformBackendKey", {
      description: "Terraform Backend Key",
      deletionWindowInDays: 7,
      enableKeyRotation: true,
    });

    new KmsAlias(this, "TerraformBackendAlias", {
      name: "alias/terraform-backend-key",
      targetKeyId: terraformKey.id,
    });
    // Create S3 Bucket
    const terraformBucket = new S3Bucket(this, "TerraformBackendBucket", {
      acl: "private",
      versioning: {
        enabled: true,
        mfaDelete: false,
      },
      serverSideEncryptionConfiguration: {
        rule: {
          applyServerSideEncryptionByDefault: {
            sseAlgorithm: "aws:kms",
            kmsMasterKeyId: terraformKey.arn,
          },
        },
      },
    });

    new S3BucketPublicAccessBlock(this, "TerraformBackendBucketPublicAccessBlock", {
      bucket: terraformBucket.id,
      blockPublicAcls: true,
      blockPublicPolicy: true,
      ignorePublicAcls: true,
      restrictPublicBuckets: true,
    });
    // Create the DynamoDB table
    const terraformLock = new DynamodbTable(this, "TerraformBackendLock", {
      name: "terraform-backend-lock",
      billingMode: "PAY_PER_REQUEST",
      hashKey: "LockID",
      attribute: [
        {
          name: "LockID",
          type: "S",
        },
      ],
    });
    // Output the bucket name
    new TerraformOutput(this, "bucket", {
      value: terraformBucket.id,
    });
    // Output the table name
    new TerraformOutput(this, "table", {
      value: terraformLock.name,
    });
    // Output the Key ID
    new TerraformOutput(this, "key_id", {
      value: terraformKey.id,
    });
  }
}

// Deploy Resources
class MyStack extends TerraformStack {
  constructor(scope: Construct, name: string) {
    super(scope, name);

    // define resources here
    new AwsProvider(this, "Aws", {
      region: "us-east-1",
    });
    new tfBackend(this, "tfBackend");
  }
}

const app = new App();
new MyStack(app, "cdktf-eval");
app.synth();
  1. cdktf synth
  2. docker run --rm -it -v ./cdktf.out/stacks:/workspace ghcr.io/aquasecurity/trivy:canary conf /workspace -d

Output:

2023-09-01T05:20:28.351Z        DEBUG   Severities: ["UNKNOWN" "LOW" "MEDIUM" "HIGH" "CRITICAL"]
2023-09-01T05:20:28.353Z        DEBUG   cache dir:  /root/.cache/trivy
2023-09-01T05:20:28.354Z        INFO    Misconfiguration scanning is enabled
2023-09-01T05:20:28.354Z        DEBUG   Failed to open the policy metadata: open /root/.cache/trivy/policy/metadata.json: no such file or directory
2023-09-01T05:20:28.354Z        INFO    Need to update the built-in policies
2023-09-01T05:20:28.354Z        INFO    Downloading the built-in policies...
2023-09-01T05:20:28.354Z        DEBUG   Using URL: ghcr.io/aquasecurity/defsec:0 to load policy bundle
44.37 KiB / 44.37 KiB [---------------------------------------------------------------------------] 100.00% 822.89 KiB p/s 300ms
2023-09-01T05:20:30.638Z        DEBUG   Digest of the built-in policies: sha256:fd5f1ce3d8efb1fe158cb41f9adb9d7c7cc5c4c863b261053c962e6d950350b3
2023-09-01T05:20:30.639Z        DEBUG   Policies successfully loaded from disk
2023-09-01T05:20:30.662Z        DEBUG   Walk the file tree rooted at '/workspace' in parallel
2023-09-01T05:20:30.665Z        DEBUG   Scanning Terraform files for misconfigurations...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4c71124]

goroutine 1 [running]:
github.com/aquasecurity/defsec/pkg/terraform.(*Block).GetMetadata(...)
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/pkg/terraform/block.go:133
github.com/aquasecurity/defsec/internal/adapters/terraform/aws/s3.isEncrypted(0x4002942fa8?)
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/internal/adapters/terraform/aws/s3/bucket.go:184 +0x94
github.com/aquasecurity/defsec/internal/adapters/terraform/aws/s3.getEncryption(_, _)
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/internal/adapters/terraform/aws/s3/bucket.go:49 +0x4d8
github.com/aquasecurity/defsec/internal/adapters/terraform/aws/s3.(*adapter).adaptBuckets(0x4002945ac8)
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/internal/adapters/terraform/aws/s3/bucket.go:21 +0x12c
github.com/aquasecurity/defsec/internal/adapters/terraform/aws/s3.Adapt(...)
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/internal/adapters/terraform/aws/s3/adapt.go:16
github.com/aquasecurity/defsec/internal/adapters/terraform/aws.Adapt({_, _, _})
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/internal/adapters/terraform/aws/adapt.go:69 +0x66c
github.com/aquasecurity/defsec/internal/adapters/terraform.Adapt({0x4002a76468, 0x1, 0x1})
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/internal/adapters/terraform/adapt.go:20 +0x3c
github.com/aquasecurity/defsec/pkg/scanners/terraform/executor.(*Executor).Execute(0x40008fb200, {0x4002a76468?, 0x1, 0x1})
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/pkg/scanners/terraform/executor/executor.go:96 +0xd4
github.com/aquasecurity/defsec/pkg/scanners/terraform.(*Scanner).ScanFSWithMetrics(0x4000960900, {0x79a3200, 0x4000e17da0}, {0x79000e0?, 0x400000dc38?}, {0x78e7fa8, 0x1})
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/pkg/scanners/terraform/scanner.go:223 +0x634
github.com/aquasecurity/defsec/pkg/scanners/terraform.(*Scanner).ScanFS(0x65ce0ae?, {0x79a3200?, 0x4000e17da0?}, {0x79000e0?, 0x400000dc38?}, {0x78e7fa8?, 0x40021fee38?})
        /home/runner/go/pkg/mod/github.com/aquasecurity/defsec@v0.92.0/pkg/scanners/terraform/scanner.go:151 +0x38
github.com/aquasecurity/trivy/pkg/misconf.(*Scanner).Scan(0x4001767590, {0x79a3200, 0x4000e17da0}, {0x79000e0?, 0x400000d650?})
        /home/runner/work/trivy/trivy/pkg/misconf/scanner.go:144 +0x124
github.com/aquasecurity/trivy/pkg/fanal/analyzer/config.(*Analyzer).PostAnalyze(0x40003d6620, {0x79a3200?, 0x4000e17da0?}, {{0x79000e0?, 0x400000d650?}, {0x9?, 0x0?}})
        /home/runner/work/trivy/trivy/pkg/fanal/analyzer/config/config.go:45 +0x38
github.com/aquasecurity/trivy/pkg/fanal/analyzer.AnalyzerGroup.PostAnalyze({{0x400063c140, 0x3, 0x4}, {0x40011c8680, 0x8, 0x8}, 0x40018103c0}, {0x79a3200, 0x4000e17da0}, 0x40017d45f0, ...)
        /home/runner/work/trivy/trivy/pkg/fanal/analyzer/analyzer.go:491 +0x234
github.com/aquasecurity/trivy/pkg/fanal/artifact/local.Artifact.Inspect({{0xffffeba4ef67, 0xa}, {0xffff8f09cbf8, 0x4001dcfaf0}, {{{0x0, 0x0, 0x0}, {0x40018123c0, 0x3, 0x4}, ...}, ...}, ...}, ...)
        /home/runner/work/trivy/trivy/pkg/fanal/artifact/local/fs.go:171 +0x454
github.com/aquasecurity/trivy/pkg/scanner.Scanner.ScanArtifact({{_, _}, {_, _}}, {_, _}, {{0x0, 0x0, 0x0}, {0x4001dcfa10, ...}, ...})
        /home/runner/work/trivy/trivy/pkg/scanner/scan.go:145 +0xa0
github.com/aquasecurity/trivy/pkg/commands/artifact.scan({_, _}, {{{0x65e03c2, 0xa}, 0x0, 0x0, 0x1, 0x0, 0x45d964b800, {0x40007d6480, ...}, ...}, ...}, ...)
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:683 +0x320
github.com/aquasecurity/trivy/pkg/commands/artifact.(*runner).scanArtifact(_, {_, _}, {{{0x65e03c2, 0xa}, 0x0, 0x0, 0x1, 0x0, 0x45d964b800, ...}, ...}, ...)
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:266 +0xa0
github.com/aquasecurity/trivy/pkg/commands/artifact.(*runner).scanFS(_, {_, _}, {{{0x65e03c2, 0xa}, 0x0, 0x0, 0x1, 0x0, 0x45d964b800, ...}, ...})
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:214 +0xa4
github.com/aquasecurity/trivy/pkg/commands/artifact.(*runner).ScanFilesystem(_, {_, _}, {{{0x65e03c2, 0xa}, 0x0, 0x0, 0x1, 0x0, 0x45d964b800, ...}, ...})
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:194 +0x1b4
github.com/aquasecurity/trivy/pkg/commands/artifact.Run({_, _}, {{{0x65e03c2, 0xa}, 0x0, 0x0, 0x1, 0x0, 0x45d964b800, {0x40007d6480, ...}, ...}, ...}, ...)
        /home/runner/work/trivy/trivy/pkg/commands/artifact/run.go:427 +0x3bc
github.com/aquasecurity/trivy/pkg/commands.NewConfigCommand.func2(0x400031c000, {0x4001d2ade0, 0x1, 0x2})
        /home/runner/work/trivy/trivy/pkg/commands/app.go:675 +0x290
github.com/spf13/cobra.(*Command).execute(0x400031c000, {0x4001d2adc0, 0x2, 0x2})
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:940 +0x5c4
github.com/spf13/cobra.(*Command).ExecuteC(0x40001df800)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x340
github.com/spf13/cobra.(*Command).Execute(0x6643523?)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992 +0x1c
main.run()
        /home/runner/work/trivy/trivy/cmd/trivy/main.go:35 +0x150
main.main()
        /home/runner/work/trivy/trivy/cmd/trivy/main.go:17 +0x1c
nikpivkin commented 1 year ago

@simar7 This problem is specific only to terraform JSON:

Due to the ambiguity of the JSON syntax, there is no way to distinguish based on the input alone between argument and nested block usage, so the JSON syntax cannot support the nested block processing mode for these arguments

Resource blocks are parsed as attributes, so when trying to find a block, nil is returned, which eventually causes panic.

simar7 commented 1 year ago

@simar7 This problem is specific only to terraform JSON:

Due to the ambiguity of the JSON syntax, there is no way to distinguish based on the input alone between argument and nested block usage, so the JSON syntax cannot support the nested block processing mode for these arguments

Resource blocks are parsed as attributes, so when trying to find a block, nil is returned, which eventually causes panic.

I see. Can we handle this case better? If nothing can be done, we can probably handle the case so that we don't panic but also are unable to support the scanning for such a case.

nikpivkin commented 1 year ago

@simar7 I think it would be difficult to add support for such cases.

The resource in JSON config is parsed to terraform block, but nested blocks and attributes are parsed as attributes, and we cannot access attributes above the first level. This makes it impossible to get the positions of these attributes for use in metadata. All we can do with these attributes is just evaluate an expression whose result is cty.Value. But all logic in adapters depends on HCL blocks and attributes, so we would need to add an interface to abstract them away. Is it worth it?

Ref:

simar7 commented 1 year ago

@simar7 I think it would be difficult to add support for such cases.

The resource in JSON config is parsed to terraform block, but nested blocks and attributes are parsed as attributes, and we cannot access attributes above the first level. This makes it impossible to get the positions of these attributes for use in metadata. All we can do with these attributes is just evaluate an expression whose result is cty.Value. But all logic in adapters depends on HCL blocks and attributes, so we would need to add an interface to abstract them away. Is it worth it?

Ref:

Fair enough. I think gracefully handling (not panicking) is the best option here. What do you think?

nikpivkin commented 1 year ago

@simar7 I think so too, I'll fix it.

simar7 commented 1 year ago

Fixed via https://github.com/aquasecurity/defsec/pull/1457