feat(checks): Improve AVD-AWS-0345

[simar7]

Discussed in #8751

^{Originally posted by simar7 April 17, 2025}
We should also be able to parse the following:

Check for specific Policy ARNs

# Provider configuration
provider "aws" {
  region = "us-west-2"
}

# Create an IAM role
resource "aws_iam_role" "example_role" {
  name = "example-role"

  assume_role_policy = jsonencode({
    Version = "2012-10-17",
    Statement = [
      {
        Effect = "Allow",
        Principal = {
          Service = "ec2.amazonaws.com"
        },
        Action = "sts:AssumeRole"
      }
    ]
  })
}

# Attach the AmazonS3FullAccess policy to the IAM role
resource "aws_iam_role_policy_attachment" "example_role_policy_attachment" {
  role       = aws_iam_role.example_role.name
  policy_arn = "arn:aws:iam::aws:policy/AmazonS3FullAccess"
}

Here "arn:aws:iam::aws:policy/AmazonS3FullAccess" equates to the following:

{
  "Version" : "2012-10-17",
  "Statement" : [
    {
      "Effect" : "Allow",
      "Action" : [
        "s3:*",
        "s3-object-lambda:*"
      ],
      "Resource" : "*"
    }
  ]
}

Check for JSON Policy docs

An example is available here https://raw.githubusercontent.com/aws-samples/aws-stepfunctions-ecs-fargate-process/d748389c6ee443389a7275f9056f712f9359b178/templates/roles.tf

[itaysk]

just to confirm, is there any change needed in the check (AVD-AWS-0345) in order to leverage this?
also, please document the new feature

[simar7]

@nikpivkin in addition to checking for named IAM policies, we also need to check for conditions such as s3:get:*, s3:put:* etc or in general s3:<verb>:*. Today the check only checks for s3:*. Here's an input that we also need to validate and flag:

resource "aws_iam_role_policy" "ecs_task_role_s3_attachment_policy" {
  name = "${local.iam_task_role_policy_name}"
  role = "${aws_iam_role.stepfunction_ecs_task_role.id}"
  policy = <<EOF
{
    "Version": "2012-10-17",
    "Statement": [
       {
            "Effect": "Allow",
            "Action": [
                "s3:put*",
                "s3:get*",
                "s3:list*"
            ],
            "Resource": "*"
        },
         {
            "Effect": "Allow",
            "Action": [
                "kinesis:DescribeStream",
                "kinesis:GetRecords",
                "kinesis:PutRecords"

            ],
            "Resource": [
                "${aws_kinesis_stream.stepfunction_ecs_kinesis_stream.arn}"
            ]
        }
    ]
}
EOF
}

I pushed some changes here aquasecurity/trivy-checks#398

[dprice-andros]

@simar7, can you clarify what the concern with something like s3:get* is? I don't understand how that is in the same category of concern as s3:* would be.

[thefire]

@simar7, can you clarify what the concern with something like s3:get* is? I don't understand how that is in the same category of concern as s3:* would be.

Agreed. The recent changes which trigger AVD-AWS-0345 when a policy has s3:Get* or s3:List* make no sense to me. If you allow s3:GetObject then also allowing the other s3:Get* actions is no less restrictive as the other actions are of little consequence once GetObject is allowed.

Please revert these latest changes to AVD-AWS-0345 as they make the check totally unusable. It should be looking for s3:* and not normal actions that simplify allowing readonly access to a bucket's objects.

@simar7 can you please explain the rationale for triggering on s3:Get* or s3:List*?

[simar7]

Please see the following blurb from our research team:

If the permission s3:Get* is attached broadly to any S3 bucket in the account, it can introduce security risks by potentially allowing unauthorized access or information disclosure from sensitive buckets.

For more details, please see our blog here:
https://www.aquasec.com/blog/shadow-roles-aws-defaults-lead-to-service-takeover/

[itaysk]

@simar7 I can't find the quote in the link. regardless, I can see how this might be confusing since the check is referring to "full access", "unrestricted acceess", etc, but is checking for any broad action (e.g Get*, right?). if we want to expand the check to alert not only for "full access" (do we?), then perhaps these cases should be treated differently (severity/description)?

[thefire]

@simar7 I agree with @itaysk. The risk description and severity between AmazonS3FullAccess and allowing s3:Get* are totally different.

And I believe the true takeaway from that article is that users should be more selective in their resources definition.

In the end what we're really protecting in a storage service like s3 is the data. So I'm at a loss to understand how s3:Get* is inherently more risky than just s3:GetObject when scoped to a single resource.

I do see how s3:List* carries higher risk, however, since s3:ListAllMyBuckets will be granted giving access to all of the account's bucket names (and it doesn't appear that this action requires anything specific under resources). But skimming through the various Get actions they all require specific resources.

[simar7]

@simar7 I can't find the quote in the link. regardless, I can see how this might be confusing since the check is referring to "full access", "unrestricted acceess", etc, but is checking for any broad action (e.g Get*, right?). if we want to expand the check to alert not only for "full access" (do we?), then perhaps these cases should be treated differently (severity/description)?

Right - thanks that makes sense. I'll separate them out. Track #8845