diff --git a/.github/workflows/semgrep-rules-test-develop.yml b/.github/workflows/semgrep-rules-test-develop.yml index 3fce6e6b84..cfe459da0c 100644 --- a/.github/workflows/semgrep-rules-test-develop.yml +++ b/.github/workflows/semgrep-rules-test-develop.yml @@ -22,6 +22,8 @@ jobs: run: rm -rf semgrep-rules/stats - name: delete fingerprints directory run: rm -rf semgrep-rules/fingerprints + - name: delete rules requiring Semgrep Pro + run: rm -rf semgrep-rules/apex - name: validate rules run: | export SEMGREP="docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src returntocorp/semgrep:develop semgrep" diff --git a/.github/workflows/semgrep-rules-test-historical.yml b/.github/workflows/semgrep-rules-test-historical.yml index 293bba3990..76b990fdd3 100644 --- a/.github/workflows/semgrep-rules-test-historical.yml +++ b/.github/workflows/semgrep-rules-test-historical.yml @@ -34,6 +34,8 @@ jobs: run: rm -rf semgrep-rules/stats - name: delete fingerprints directory run: rm -rf semgrep-rules/fingerprints + - name: delete rules requiring Semgrep Pro + run: rm -rf semgrep-rules/apex - name: grab historical semgrep version env: GH_TOKEN: ${{ github.token }} diff --git a/.github/workflows/semgrep-rules-test.yml b/.github/workflows/semgrep-rules-test.yml index ff3bb10668..4264634ed3 100644 --- a/.github/workflows/semgrep-rules-test.yml +++ b/.github/workflows/semgrep-rules-test.yml @@ -23,6 +23,8 @@ jobs: run: rm -rf stats - name: remove fingerprints from testing run: rm -rf fingerprints + - name: remove rules requiring Semgrep Pro + run: rm -rf apex - name: validate rules run: semgrep --validate --config . - name: run semgrep diff --git a/generic/ci/audit/changed-semgrepignore.generic b/generic/ci/audit/changed-semgrepignore.generic deleted file mode 100644 index 18425fb942..0000000000 --- a/generic/ci/audit/changed-semgrepignore.generic +++ /dev/null @@ -1,25 +0,0 @@ -# Ignore git items -.gitignore -.git/ -:include .gitignore - -# Common large paths -node_modules/ -build/ -dist/ -vendor/ -.env/ -.venv/ -.tox/ -*.min.js - -# Common test paths -test/ -tests/ -*_test.go - -# Semgrep rules folder -.semgrep - -# Semgrep-action log folder -.semgrep_logs/ diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-ajax-no-auth-and-auth-hooks-audit.php b/php/wordpress-plugins/security/audit/wp-ajax-no-auth-and-auth-hooks-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-ajax-no-auth-and-auth-hooks-audit.php rename to php/wordpress-plugins/security/audit/wp-ajax-no-auth-and-auth-hooks-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-authorisation-checks-audit.php b/php/wordpress-plugins/security/audit/wp-authorisation-checks-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-authorisation-checks-audit.php rename to php/wordpress-plugins/security/audit/wp-authorisation-checks-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-code-execution-audit.php b/php/wordpress-plugins/security/audit/wp-code-execution-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-code-execution-audit.php rename to php/wordpress-plugins/security/audit/wp-code-execution-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-command-execution-audit.php b/php/wordpress-plugins/security/audit/wp-command-execution-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-command-execution-audit.php rename to php/wordpress-plugins/security/audit/wp-command-execution-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-csrf-audit.php b/php/wordpress-plugins/security/audit/wp-csrf-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-csrf-audit.php rename to php/wordpress-plugins/security/audit/wp-csrf-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-file-download-audit.php b/php/wordpress-plugins/security/audit/wp-file-download-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-file-download-audit.php rename to php/wordpress-plugins/security/audit/wp-file-download-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-file-inclusion-audit.php b/php/wordpress-plugins/security/audit/wp-file-inclusion-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-file-inclusion-audit.php rename to php/wordpress-plugins/security/audit/wp-file-inclusion-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-file-manipulation-audit.php b/php/wordpress-plugins/security/audit/wp-file-manipulation-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-file-manipulation-audit.php rename to php/wordpress-plugins/security/audit/wp-file-manipulation-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-open-redirect-audit.php b/php/wordpress-plugins/security/audit/wp-open-redirect-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-open-redirect-audit.php rename to php/wordpress-plugins/security/audit/wp-open-redirect-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-php-object-injection-audit.php b/php/wordpress-plugins/security/audit/wp-php-object-injection-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-php-object-injection-audit.php rename to php/wordpress-plugins/security/audit/wp-php-object-injection-audit.php diff --git a/php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-sql-injection-audit.php b/php/wordpress-plugins/security/audit/wp-sql-injection-audit.php similarity index 100% rename from php/wordpress-plugins/security/audit/tests/wp-content/plugins/wp-sql-injection-audit.php rename to php/wordpress-plugins/security/audit/wp-sql-injection-audit.php diff --git a/python/jwt/security/unverified-jwt-decode.fixed.py b/python/jwt/security/unverified-jwt-decode.fixed.py new file mode 100644 index 0000000000..b441b9e599 --- /dev/null +++ b/python/jwt/security/unverified-jwt-decode.fixed.py @@ -0,0 +1,33 @@ +# cf. https://github.com/we45/Vulnerable-Flask-App/blob/752ee16087c0bfb79073f68802d907569a1f0df7/app/app.py#L96 + +import jwt +from jwt.exceptions import DecodeError, MissingRequiredClaimError, InvalidKeyError + +def tests(token): + # ruleid:unverified-jwt-decode + jwt.decode(encoded, key, options={"verify_signature": True}) + + # ruleid:unverified-jwt-decode + opts = {"verify_signature": True} + jwt.decode(encoded, key, options=opts) + + a_false_boolean = False + # ruleid:unverified-jwt-decode + opts2 = {"verify_signature": True} + jwt.decode(encoded, key, options=opts2) + + # ok:unverified-jwt-decode + jwt.decode(encoded, key, options={"verify_signature": True}) + + opts = {"verify_signature": True} + # ok:unverified-jwt-decode + jwt.decode(encoded, key, options=opts) + + a_false_boolean = True + opts2 = {"verify_signature": a_false_boolean} + # ok:unverified-jwt-decode + jwt.decode(encoded, key, options=opts2) + + # ok:unverified-jwt-decode + jwt.decode(encoded, key) + diff --git a/python/jwt/security/unverified-jwt-decode.py b/python/jwt/security/unverified-jwt-decode.py index 2aef900ef7..a412e5cea5 100644 --- a/python/jwt/security/unverified-jwt-decode.py +++ b/python/jwt/security/unverified-jwt-decode.py @@ -3,21 +3,31 @@ import jwt from jwt.exceptions import DecodeError, MissingRequiredClaimError, InvalidKeyError -def verify_jwt(token): - try: - # ok:unverified-jwt-decode - decoded = jwt.decode(token, app.config['SECRET_KEY_HMAC'], verify=True, issuer = 'we45', leeway=10, algorithms=['HS256']) - print("JWT Token from API: {0}".format(decoded)) - return True - except DecodeError: - print("Error in decoding token") - return False - except MissingRequiredClaimError as e: - print('Claim required is missing: {0}'.format(e)) - return False - -def insecure_verify(token): +def tests(token): # ruleid:unverified-jwt-decode - decoded = jwt.decode(token, verify = False) - print(decoded) - return True + jwt.decode(encoded, key, options={"verify_signature": False}) + + # ruleid:unverified-jwt-decode + opts = {"verify_signature": False} + jwt.decode(encoded, key, options=opts) + + a_false_boolean = False + # ruleid:unverified-jwt-decode + opts2 = {"verify_signature": a_false_boolean} + jwt.decode(encoded, key, options=opts2) + + # ok:unverified-jwt-decode + jwt.decode(encoded, key, options={"verify_signature": True}) + + opts = {"verify_signature": True} + # ok:unverified-jwt-decode + jwt.decode(encoded, key, options=opts) + + a_false_boolean = True + opts2 = {"verify_signature": a_false_boolean} + # ok:unverified-jwt-decode + jwt.decode(encoded, key, options=opts2) + + # ok:unverified-jwt-decode + jwt.decode(encoded, key) + diff --git a/python/jwt/security/unverified-jwt-decode.yaml b/python/jwt/security/unverified-jwt-decode.yaml index f1d30c69ed..3452beb6b1 100644 --- a/python/jwt/security/unverified-jwt-decode.yaml +++ b/python/jwt/security/unverified-jwt-decode.yaml @@ -1,7 +1,25 @@ rules: - id: unverified-jwt-decode - pattern: | - jwt.decode(..., verify=False, ...) + patterns: + - pattern-either: + - patterns: + - pattern: | + jwt.decode(..., options={..., "verify_signature": $BOOL, ...}, ...) + - metavariable-pattern: + metavariable: $BOOL + pattern: | + False + - focus-metavariable: $BOOL + - patterns: + - pattern: | + $OPTS = {..., "verify_signature": $BOOL, ...} + ... + jwt.decode(..., options=$OPTS, ...) + - metavariable-pattern: + metavariable: $BOOL + pattern: | + False + - focus-metavariable: $BOOL message: >- Detected JWT token decoded with 'verify=False'. This bypasses any integrity checks for the token which means the token could be tampered with by @@ -24,9 +42,8 @@ rules: likelihood: MEDIUM impact: MEDIUM confidence: MEDIUM - fix-regex: - regex: (verify\s*=\s*)False - replacement: \1True + fix: | + True severity: ERROR languages: - python diff --git a/scripts/run-tests b/scripts/run-tests index 887aa79908..0a83e63a59 100755 --- a/scripts/run-tests +++ b/scripts/run-tests @@ -40,9 +40,12 @@ fi # may contain .yml files that are not Semgrep rules and would result # in errors. # +# Skipping the "Apex" folder because it will require splitting test logic +# to run Semgrep OSS and Semgrep Pro with different expected results. +# set_rule_folders() { rule_folders=$(find . -mindepth 1 -maxdepth 1 -type d \ - | grep -v '^./\(\..*\|stats\|trusted_python\|fingerprints\|scripts\|libsonnet\)/\?$' \ + | grep -v '^./\(\..*\|stats\|trusted_python\|fingerprints\|scripts\|libsonnet\|apex\)/\?$' \ | sort) if [[ -z "$rule_folders" ]]; then error "Cannot find any rule folders to scan in $(pwd)" diff --git a/terraform/aws/correctness/lambda-permission-logs-missing-arn-asterisk.tf b/terraform/aws/correctness/lambda-permission-logs-missing-arn-asterisk.tf new file mode 100644 index 0000000000..2ec258de02 --- /dev/null +++ b/terraform/aws/correctness/lambda-permission-logs-missing-arn-asterisk.tf @@ -0,0 +1,31 @@ +resource "aws_lambda_permission" "allow_cloudwatch" { + statement_id = "${var.name}-allow-execution-from-cloudwatch" + action = "lambda:InvokeFunction" + function_name = aws_lambda_function.lambda_function.function_name + principal = "logs.amazonaws.com" + # ruleid: lambda-permission-logs-missing-arn-asterisk + source_arn = "arn:aws:logs:us-west-2:${data.aws_caller_identity.current.account_id}:log-group:${var.log_group_name}" + + depends_on = [aws_lambda_function.lambda_function] +} + +resource "aws_lambda_permission" "allow_cloudwatch" { + statement_id = "${var.name}-allow-execution-from-cloudwatch" + action = "lambda:InvokeFunction" + function_name = aws_lambda_function.lambda_function.function_name + principal = "logs.amazonaws.com" + # ok: lambda-permission-logs-missing-arn-asterisk + source_arn = "arn:aws:logs:us-west-2:${data.aws_caller_identity.current.account_id}:log-group:${var.log_group_name}:*" + + depends_on = [aws_lambda_function.lambda_function] +} + +resource "aws_lambda_permission" "allow_cloudwatch" { + statement_id = "AllowExecutionFromCloudWatch" + action = "lambda:InvokeFunction" + function_name = aws_lambda_function.test_lambda.function_name + principal = "events.amazonaws.com" + # ok: lambda-permission-logs-missing-arn-asterisk + source_arn = "arn:aws:events:eu-west-1:111122223333:rule/RunDaily" + qualifier = aws_lambda_alias.test_alias.name +} diff --git a/terraform/aws/correctness/lambda-permission-logs-missing-arn-asterisk.yaml b/terraform/aws/correctness/lambda-permission-logs-missing-arn-asterisk.yaml new file mode 100644 index 0000000000..6400b2d3de --- /dev/null +++ b/terraform/aws/correctness/lambda-permission-logs-missing-arn-asterisk.yaml @@ -0,0 +1,25 @@ +rules: +- id: lambda-permission-logs-missing-arn-asterisk + severity: WARNING + languages: [hcl] + message: "The `source_arn` field needs to end with an asterisk, like this: `:*` Without this, the `aws_lambda_permission` resource '$NAME' will not be created. Add the asterisk to the end of the arn. x $ARN" + metadata: + category: correctness + references: + - https://github.com/hashicorp/terraform-provider-aws/issues/14630 + technology: + - aws + - terraform + - aws-lambda + patterns: + - pattern-inside: | + resource "aws_lambda_permission" "$NAME" { ... } + - pattern: | + source_arn = $ARN + - metavariable-pattern: + metavariable: $ARN + patterns: + - pattern-regex: + arn:aws:logs.* + - pattern-not-regex: >- + arn:aws:logs:.*:\* diff --git a/terraform/aws/correctness/lambda-redundant-field-with-image.tf b/terraform/aws/correctness/lambda-redundant-field-with-image.tf new file mode 100644 index 0000000000..92ba19470a --- /dev/null +++ b/terraform/aws/correctness/lambda-redundant-field-with-image.tf @@ -0,0 +1,112 @@ +resource "aws_lambda_function" "forward_to_sns" { + function_name = "${var.name}-cloudwatch-forward-to-sns" + role = aws_iam_role.lambda_to_sns.arn + timeout = 120 + package_type = "Image" + image_uri = "image/goes/here" + # ruleid: lambda-redundant-field-with-image + handler = "main.lambda_handler" + # ruleid: lambda-redundant-field-with-image + runtime = "python3.9" + + environment { + variables = { + SNS_TOPIC_ARN = aws_sns_topic.sns_topic.arn + AWS_REGION_OF_SNS_TOPIC = var.region + } + } + depends_on = [aws_iam_role.lambda_to_sns] +} + +resource "aws_lambda_function" "forward_to_sns" { + function_name = "${var.name}-cloudwatch-forward-to-sns" + role = aws_iam_role.lambda_to_sns.arn + timeout = 120 + package_type = "Image" + image_uri = "image/goes/here" + # ruleid: lambda-redundant-field-with-image + handler = "main.lambda_handler" + + environment { + variables = { + SNS_TOPIC_ARN = aws_sns_topic.sns_topic.arn + AWS_REGION_OF_SNS_TOPIC = var.region + } + } + depends_on = [aws_iam_role.lambda_to_sns] +} + +resource "aws_lambda_function" "forward_to_sns" { + function_name = "${var.name}-cloudwatch-forward-to-sns" + role = aws_iam_role.lambda_to_sns.arn + timeout = 120 + package_type = "Image" + image_uri = "image/goes/here" + # ruleid: lambda-redundant-field-with-image + runtime = "python3.9" + + environment { + variables = { + SNS_TOPIC_ARN = aws_sns_topic.sns_topic.arn + AWS_REGION_OF_SNS_TOPIC = var.region + } + } + depends_on = [aws_iam_role.lambda_to_sns] +} + +resource "aws_lambda_function" "forward_to_sns" { + function_name = "${var.name}-cloudwatch-forward-to-sns" + role = aws_iam_role.lambda_to_sns.arn + timeout = 120 + package_type = "Image" + # ok: lambda-redundant-field-with-image + image_uri = "image/goes/here" + + environment { + variables = { + SNS_TOPIC_ARN = aws_sns_topic.sns_topic.arn + AWS_REGION_OF_SNS_TOPIC = var.region + } + } + depends_on = [aws_iam_role.lambda_to_sns] +} + +resource "aws_lambda_function" "forward_to_sns" { + function_name = "${var.name}-cloudwatch-forward-to-sns" + role = aws_iam_role.lambda_to_sns.arn + timeout = 120 + package_type = "Zip" + image_uri = "image/goes/here" + # ok: lambda-redundant-field-with-image + handler = "main.lambda_handler" + # ok: lambda-redundant-field-with-image + runtime = "python3.9" + + environment { + variables = { + SNS_TOPIC_ARN = aws_sns_topic.sns_topic.arn + AWS_REGION_OF_SNS_TOPIC = var.region + } + } + depends_on = [aws_iam_role.lambda_to_sns] +} + + +resource "aws_lambda_function" "forward_to_sns" { + function_name = "${var.name}-cloudwatch-forward-to-sns" + role = aws_iam_role.lambda_to_sns.arn + timeout = 120 + image_uri = "image/goes/here" + # ok: lambda-redundant-field-with-image + handler = "main.lambda_handler" + # ok: lambda-redundant-field-with-image + runtime = "python3.9" + + environment { + variables = { + SNS_TOPIC_ARN = aws_sns_topic.sns_topic.arn + AWS_REGION_OF_SNS_TOPIC = var.region + } + } + depends_on = [aws_iam_role.lambda_to_sns] +} diff --git a/terraform/aws/correctness/lambda-redundant-field-with-image.yaml b/terraform/aws/correctness/lambda-redundant-field-with-image.yaml new file mode 100644 index 0000000000..713fe3ac24 --- /dev/null +++ b/terraform/aws/correctness/lambda-redundant-field-with-image.yaml @@ -0,0 +1,23 @@ +rules: +- id: lambda-redundant-field-with-image + severity: WARNING + languages: [hcl] + message: 'When using the AWS Lambda "Image" package_type, `runtime` and `handler` are not necessary for Lambda to understand how to run the code. These are built into the container image. Including `runtime` or `handler` with an "Image" `package_type` will result in an error on `terraform apply`. Remove these redundant fields.' + metadata: + category: correctness + references: + - https://stackoverflow.com/questions/72771366/why-do-i-get-error-handler-and-runtime-must-be-set-when-packagetype-is-zip-whe + technology: + - aws + - terraform + - aws-lambda + patterns: + - pattern-inside: | + resource "aws_lambda_function" $NAME { + ... + package_type = "Image" + } + - pattern-either: + - pattern: handler = ... + - pattern: runtime = ... + diff --git a/terraform/aws/correctness/subscription-filter-missing-depends.tf b/terraform/aws/correctness/subscription-filter-missing-depends.tf new file mode 100644 index 0000000000..56511986ec --- /dev/null +++ b/terraform/aws/correctness/subscription-filter-missing-depends.tf @@ -0,0 +1,37 @@ +# ruleid: subscription-filter-missing-depends +resource "aws_cloudwatch_log_subscription_filter" "log_subscription_filter" { + name = var.name + log_group_name = var.log_group_name + filter_pattern = var.subscription_filter_pattern + destination_arn = aws_lambda_function.forward_to_sns.arn +} + +# ruleid: subscription-filter-missing-depends +resource "aws_cloudwatch_log_subscription_filter" "log_subscription_filter" { + name = var.name + log_group_name = var.log_group_name + filter_pattern = var.subscription_filter_pattern + destination_arn = aws_lambda_function.forward_to_sns.arn + + depends_on = [aws_lambda_function.forward_to_sns] +} + +# ok: subscription-filter-missing-depends +resource "aws_cloudwatch_log_subscription_filter" "log_subscription_filter" { + name = var.name + log_group_name = var.log_group_name + filter_pattern = var.subscription_filter_pattern + destination_arn = aws_lambda_function.forward_to_sns.arn + + depends_on = [aws_lambda_permission.allow_cloudwatch] +} + +# ok: subscription-filter-missing-depends +resource "aws_cloudwatch_log_subscription_filter" "log_subscription_filter" { + name = var.name + log_group_name = var.log_group_name + filter_pattern = var.subscription_filter_pattern + destination_arn = aws_lambda_function.forward_to_sns.arn + + depends_on = [aws_lambda_permission.allow_cloudwatch, aws_lambda_function.forward_to_sns] +} diff --git a/terraform/aws/correctness/subscription-filter-missing-depends.yaml b/terraform/aws/correctness/subscription-filter-missing-depends.yaml new file mode 100644 index 0000000000..aff5e04952 --- /dev/null +++ b/terraform/aws/correctness/subscription-filter-missing-depends.yaml @@ -0,0 +1,26 @@ +rules: +- id: subscription-filter-missing-depends + severity: WARNING + languages: [hcl] + message: 'The `aws_cloudwatch_log_subscription_filter` resource "$NAME" needs a `depends_on` clause on the `aws_lambda_permission`, otherwise Terraform may try to create these out-of-order and fail.' + metadata: + category: correctness + references: + - https://stackoverflow.com/questions/38407660/terraform-configuring-cloudwatch-log-subscription-delivery-to-lambda/38428834#38428834 + technology: + - aws + - terraform + - aws-lambda + - cloudwatch + confidence: MEDIUM + patterns: + - pattern: | + resource "aws_cloudwatch_log_subscription_filter" $NAME { + ... + destination_arn = aws_lambda_function.$LAMBDA_NAME.arn + } + - pattern-not-inside: | + resource "aws_cloudwatch_log_subscription_filter" $NAME { + ... + depends_on = [..., aws_lambda_permission.$PERMISSION_NAME, ...] + }