Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nikpivkin opened this issue Sep 1, 2023 · 6 comments
Closed
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. scan/misconfiguration Issues relating to misconfiguration scanning
Milestone

Comments

@nikpivkin
Copy link
Contributor

nikpivkin commented Sep 1, 2023

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 nikpivkin added kind/bug Categorizes issue or PR as related to a bug. scan/misconfiguration Issues relating to misconfiguration scanning labels Sep 1, 2023
@simar7 simar7 added this to the v0.46.0 milestone Sep 6, 2023
@nikpivkin nikpivkin self-assigned this Sep 13, 2023
@nikpivkin
Copy link
Contributor Author

@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
Copy link
Member

simar7 commented Sep 15, 2023

@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
Copy link
Contributor Author

@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
Copy link
Member

simar7 commented Sep 18, 2023

@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
Copy link
Contributor Author

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

@simar7
Copy link
Member

simar7 commented Sep 20, 2023

Fixed via aquasecurity/defsec#1457

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. scan/misconfiguration Issues relating to misconfiguration scanning
Projects
None yet
Development

No branches or pull requests

2 participants