fix(custom-resources-handlers): s3 deployment handler log injection vulnerability #28599
Conversation
github-actions
bot
added
bug
There was a problem hiding this comment.
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.
|
Exemption Request. |
aws-cdk-automation
added
the
pr-linter/exemption-requested
aws-cdk-automation
added
the
pr/needs-maintainer-review
| @@ -108,7 +109,7 @@ def cfn_error(message=None): | |||
| physical_id = "aws.cdk.s3deployment.%s" % str(uuid4()) | |||
| else: | |||
| if not physical_id: | |||
| cfn_error("invalid request: request type is '%s' but 'PhysicalResourceId' is not defined" % {request_type}) | |||
| cfn_error("invalid request: request type is '%s' but 'PhysicalResourceId' is not defined" % request_type) | |||
There was a problem hiding this comment.
Just for my own understanding, is this to fix CWE-93? If so, how does this actually fix it?
There was a problem hiding this comment.
The fix is at line 34: using encode() to sanitize the logged error message.
There was a problem hiding this comment.
Oh I was thinking that was only to fix CWE-117. Does using encode() fix both vulnerabilities then?
There was a problem hiding this comment.
It should fix both (CWE-117 and CWE-93 are related vulnerabilities), as suggested here.
Not sure if AWS Inspector requires an implementation with urllib.parse.quote.
paulhcsun
added
the
pr-linter/exempt-integ-test
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
@lpizzinidev It looks like the snapshot for |
There was a problem hiding this comment.
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.
lpizzinidev
changed the title
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
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). |
The
bucket-deployment-handlerresults vulnerable to CWE-117 and CWE-93 according to AWS Inspector.This fix mitigates the vulnerability by sanitizing the logged
messageas suggested on Veracode.Note
Inspector suggestion of using
urllib.parse.quotewould produce unreadable messages, so I opted forencoded.Closes #28469.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license