chore(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30746

GavinZZ · 2024-07-03T20:50:54Z

Issue # (if applicable)

Reason for this change

Original PR #30225

Currently the s3_dest and old_s3_dest are logged as received. AWS inspector has identified as HIGH findings(CWE-117,93 - Log injection) in the lambda code.

Description of changes

We are sanitizing the message before logging to mitigate the CWE-117,93 - Log injection vulnerabilites.

Description of how you validated changes

Run all the existing integ test for s3-deployment custom resource and checked the AWS inspector if the finding still exists.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

TheRealAmazonKendra · 2024-07-04T00:31:06Z

This looks good. I'll approve it as soon as our other custom resources stop ruining my life.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

… handler

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

...-source.js.snapshot/DynamoImportSourceIntegTestDefaultTestDeployAssert540810E4.template.json

shikha372 · 2024-07-18T18:55:15Z

@Mergifyio update

mergify · 2024-07-18T18:55:23Z

update

✅ Branch has been successfully updated

mergify · 2024-07-19T17:10:35Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-07-19T17:39:27Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: cf545ed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-07-19T17:39:47Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2024-07-19T17:40:02Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

…nt handler (aws#30746) ### Issue # (if applicable) Closes aws#30211. ### Reason for this change Original PR aws#30225 Currently the `s3_dest` and `old_s3_dest` are logged as received. AWS inspector has identified as HIGH findings(CWE-[117](https://cwe.mitre.org/data/definitions/117.html),[93](https://cwe.mitre.org/data/definitions/93.html) - Log injection) in the lambda code. ### Description of changes We are sanitizing the message before logging to mitigate the CWE-[117](https://cwe.mitre.org/data/definitions/117.html),[93](https://cwe.mitre.org/data/definitions/93.html) - Log injection vulnerabilites. ### Description of how you validated changes Run all the existing integ test for s3-deployment custom resource and checked the AWS inspector if the finding still exists. ![image](https://github.com/aws/aws-cdk/assets/4015201/909ac257-6b7d-4308-8b16-6b98a4ec2fc1) ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation · 2024-07-25T17:43:26Z

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

github-actions bot added the bug This issue is a bug. label Jul 3, 2024

aws-cdk-automation requested a review from a team July 3, 2024 20:51

github-actions bot added effort/small Small work item – less than a day of effort p2 labels Jul 3, 2024

GavinZZ mentioned this pull request Jul 3, 2024

fix(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30225

Closed

1 task

mergify bot added the contribution/core This is a PR that came from AWS. label Jul 3, 2024

aws-cdk-automation previously requested changes Jul 3, 2024

View reviewed changes

GavinZZ marked this pull request as ready for review July 3, 2024 22:49

GavinZZ added the pr/do-not-merge This PR should not be merged at this time. label Jul 3, 2024

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jul 3, 2024

GavinZZ force-pushed the yuanhaoz/securityfix branch from 04fb900 to 54c9735 Compare July 5, 2024 17:28

aws-cdk-automation previously requested changes Jul 5, 2024

View reviewed changes

github-actions bot mentioned this pull request Jul 8, 2024

Weekly PR metrics report #30783

Closed

GavinZZ added 2 commits July 10, 2024 15:13

fix(s3-deployment): sanitize log message in CustomCDKBucketDeployment…

45fecb2

… handler

Update sanitize message with unit test

3fc9285

GavinZZ force-pushed the yuanhaoz/securityfix branch from fa48b84 to d5df05f Compare July 10, 2024 22:13

aws-cdk-automation previously requested changes Jul 10, 2024

View reviewed changes

GavinZZ changed the title ~~fix(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler~~ chore(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler Jul 10, 2024

Update integ tests

b6c7846

GavinZZ force-pushed the yuanhaoz/securityfix branch from d5df05f to b6c7846 Compare July 11, 2024 00:01

GavinZZ added 2 commits July 11, 2024 13:17

Update integ tests

b916c51

Merge branch 'main' into yuanhaoz/securityfix

3bc877a

moelasmar reviewed Jul 12, 2024

View reviewed changes

...-source.js.snapshot/DynamoImportSourceIntegTestDefaultTestDeployAssert540810E4.template.json Show resolved Hide resolved

godwingrs22 mentioned this pull request Jul 16, 2024

CDK custom resource CustomCDKBucketDeployment: SecurityHub HIGH notification: CWE-117,93 - Log injection #30211

Closed

Merge branch 'main' into yuanhaoz/securityfix

896a2e7

shikha372 approved these changes Jul 18, 2024

View reviewed changes

Merge branch 'main' into yuanhaoz/securityfix

2967aff

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jul 18, 2024

GavinZZ removed the pr/do-not-merge This PR should not be merged at this time. label Jul 19, 2024

Merge branch 'main' into yuanhaoz/securityfix

cf545ed

mergify bot merged commit 836c507 into main Jul 19, 2024
11 of 12 checks passed

mergify bot deleted the yuanhaoz/securityfix branch July 19, 2024 17:39

aws locked as resolved and limited conversation to collaborators Jul 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30746

chore(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30746

GavinZZ commented Jul 3, 2024

aws-cdk-automation left a comment

TheRealAmazonKendra commented Jul 4, 2024

aws-cdk-automation left a comment

aws-cdk-automation left a comment

shikha372 commented Jul 18, 2024

mergify bot commented Jul 18, 2024

mergify bot commented Jul 19, 2024

aws-cdk-automation commented Jul 19, 2024

mergify bot commented Jul 19, 2024

github-actions bot commented Jul 19, 2024

aws-cdk-automation commented Jul 25, 2024

chore(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30746

chore(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30746

Conversation

GavinZZ commented Jul 3, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

TheRealAmazonKendra commented Jul 4, 2024

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

shikha372 commented Jul 18, 2024

mergify bot commented Jul 18, 2024

✅ Branch has been successfully updated

mergify bot commented Jul 19, 2024

aws-cdk-automation commented Jul 19, 2024

AWS CodeBuild CI Report

mergify bot commented Jul 19, 2024

github-actions bot commented Jul 19, 2024

aws-cdk-automation commented Jul 25, 2024