Skip to content

Commit

Permalink
fix(stepfunctions-tasks): custom resource uses subprocess with Shell=…
Browse files Browse the repository at this point in the history
…true (#22752)

Using `Shell=true` in `subprocess` functions is considered a [security vulnerability](https://cwe.mitre.org/data/definitions/78.html). To avoid using this argument, the command has to be passed as an array rather than a string.

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
otaviomacedo authored Nov 2, 2022
1 parent a36f2f0 commit bd056d1
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
def handler(event, context):
logger = logging.getLogger()
logger.setLevel(logging.INFO)
command = f"/opt/awscli/aws emr-containers update-role-trust-policy --cluster-name {event['ResourceProperties']['eksClusterId']} --namespace {event['ResourceProperties']['eksNamespace']} --role-name {event['ResourceProperties']['roleName']}"
command = ["/opt/awscli/aws", "emr-containers", "update-role-trust-policy", "--cluster-name", f"{event['ResourceProperties']['eksClusterId']}", "--namespace", f"{event['ResourceProperties']['eksNamespace']}", "--role-name", f"{event['ResourceProperties']['roleName']}"]
if event['RequestType'] == 'Create' or event['RequestType'] == 'Update' :
try:
res = sp.check_output(command, shell=True)
res = sp.check_output(command)
logger.info(f"Successfully ran {command}")
except Exception as e:
logger.info(f"ERROR: {str(e)}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
def handler(event, context):
logger = logging.getLogger()
logger.setLevel(logging.INFO)
command = f"/opt/awscli/aws emr-containers update-role-trust-policy --cluster-name {event['ResourceProperties']['eksClusterId']} --namespace {event['ResourceProperties']['eksNamespace']} --role-name {event['ResourceProperties']['roleName']}"
command = ["/opt/awscli/aws", "emr-containers", "update-role-trust-policy", "--cluster-name", f"{event['ResourceProperties']['eksClusterId']}", "--namespace", f"{event['ResourceProperties']['eksNamespace']}", "--role-name", f"{event['ResourceProperties']['roleName']}"]
if event['RequestType'] == 'Create' or event['RequestType'] == 'Update' :
try:
res = sp.check_output(command, shell=True)
res = sp.check_output(command)
logger.info(f"Successfully ran {command}")
except Exception as e:
logger.info(f"ERROR: {str(e)}")
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,15 @@
}
}
},
"de2da116e1de2db20dc2bc88a1e97df050dde2917a4122674e054e87ee53e334": {
"3c25783c134c6817b53033bdc57fc404bda6ba93392fcc7d3ca4d92bd072351f": {
"source": {
"path": "asset.de2da116e1de2db20dc2bc88a1e97df050dde2917a4122674e054e87ee53e334",
"path": "asset.3c25783c134c6817b53033bdc57fc404bda6ba93392fcc7d3ca4d92bd072351f",
"packaging": "zip"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "de2da116e1de2db20dc2bc88a1e97df050dde2917a4122674e054e87ee53e334.zip",
"objectKey": "3c25783c134c6817b53033bdc57fc404bda6ba93392fcc7d3ca4d92bd072351f.zip",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down Expand Up @@ -131,15 +131,15 @@
}
}
},
"da24149302db350010268fabd5306eace023f9bd5f26749e3160db3c82e2a2f2": {
"edffa811caefa503489be039d857f8a7abd33dadadc71461d81240ab0ddca7bc": {
"source": {
"path": "aws-stepfunctions-tasks-emr-containers-start-job-run-test.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "da24149302db350010268fabd5306eace023f9bd5f26749e3160db3c82e2a2f2.json",
"objectKey": "edffa811caefa503489be039d857f8a7abd33dadadc71461d81240ab0ddca7bc.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1496,7 +1496,7 @@
"S3Bucket": {
"Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
},
"S3Key": "de2da116e1de2db20dc2bc88a1e97df050dde2917a4122674e054e87ee53e334.zip"
"S3Key": "3c25783c134c6817b53033bdc57fc404bda6ba93392fcc7d3ca4d92bd072351f.zip"
},
"Role": {
"Fn::GetAtt": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/da24149302db350010268fabd5306eace023f9bd5f26749e3160db3c82e2a2f2.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/edffa811caefa503489be039d857f8a7abd33dadadc71461d81240ab0ddca7bc.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3952,7 +3952,7 @@
"s3Bucket": {
"Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
},
"s3Key": "de2da116e1de2db20dc2bc88a1e97df050dde2917a4122674e054e87ee53e334.zip"
"s3Key": "3c25783c134c6817b53033bdc57fc404bda6ba93392fcc7d3ca4d92bd072351f.zip"
},
"role": {
"Fn::GetAtt": [
Expand Down

0 comments on commit bd056d1

Please sign in to comment.