Skip to content
New issue

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

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

Already on GitHub? # to your account

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

Closed
wants to merge 1 commit into from

Conversation

godwingrs22
Copy link
Member

@godwingrs22 godwingrs22 commented May 15, 2024

Issue # (if applicable)

Closes #30211.

Reason for this change

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.

image

Checklist


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

@aws-cdk-automation aws-cdk-automation requested a review from a team May 15, 2024 23:20
@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels May 15, 2024
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label May 15, 2024
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@godwingrs22 godwingrs22 added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label May 15, 2024
@godwingrs22
Copy link
Member Author

Exemption Request: Changes only related to logging and doesn't require integ test.

@godwingrs22 godwingrs22 marked this pull request as ready for review May 15, 2024 23:25
@aws-cdk-automation aws-cdk-automation dismissed their stale review May 15, 2024 23:25

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

Copy link
Contributor

@shikha372 shikha372 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.. will wait for current build to pass

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 0acfeea
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@shikha372 shikha372 self-assigned this May 16, 2024
@shikha372
Copy link
Contributor

Below five failing tests to be addressed . cc @godwingrs22

@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-codepipeline-actions/test/integ.pipeline-elastic-beanstalk-deploy.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.import-source.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.environment-file.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-servicecatalog/test/integ.product.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-servicecatalog/test/integ.two-products.js

@GavinZZ
Copy link
Contributor

GavinZZ commented Jul 3, 2024

Closing this PR as there's too many conflicts and failed integ tests. Going to create a new PR for this issue. #30746

@GavinZZ GavinZZ closed this Jul 3, 2024
mergify bot pushed a commit that referenced this pull request Jul 19, 2024
…nt handler (#30746)

### Issue # (if applicable)

Closes #30211.

### 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](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*
obraafo pushed a commit to obraafo/aws-cdk that referenced this pull request Jul 25, 2024
…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*
@godwingrs22 godwingrs22 deleted the secruityfix branch August 9, 2024 22:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug This issue is a bug. contribution/core This is a PR that came from AWS. effort/small Small work item – less than a day of effort p1 pr-linter/exempt-integ-test The PR linter will not require integ test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CDK custom resource CustomCDKBucketDeployment: SecurityHub HIGH notification: CWE-117,93 - Log injection
4 participants