Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Test for hardcoded credentials, CWE798 #1167

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/build-publish-image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ jobs:
ref: ${{ github.event_name == 'release' && github.ref || env.RELEASE_TAG }}

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@988b5a0280414f521da01fcc63a27aeeb4b104db # v3
uses: docker/setup-buildx-action@d70bba72b1f3fd22344832f00baa16ece964efeb # v3
Copy link
Member

Choose a reason for hiding this comment

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

Undo, updates to these versions are managed by dependabot.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure what, but something was updating these versions forcefully on my repo... Maybe I have some conflicting dependabot config, not sure. Will fix it.


- name: Log in to GitHub Container Registry
uses: docker/login-action@9780b0c442fbb1117ed29e0efdff1e18412f7567 # v3
uses: docker/login-action@0d4c9c5ea7693da7b068278f7b52bda2a190a446 # v3
Copy link
Member

Choose a reason for hiding this comment

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

Undo, updates to these versions are managed by dependabot.

with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Install Cosign
uses: sigstore/cosign-installer@4959ce089c160fddf62f7b42464195ba1a56d382 # v3.6.0
uses: sigstore/cosign-installer@59acb6260d9c0ba8f4a2f9d9b48431a222b68e20 # v3.5.0
Copy link
Member

Choose a reason for hiding this comment

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

Undo, updates to these versions are managed by dependabot.

with:
cosign-release: 'v2.2.2'

Expand All @@ -51,7 +51,7 @@ jobs:

- name: Build and push Docker image
id: build-and-push
uses: docker/build-push-action@16ebe778df0e7752d2cfcbd924afdbbd89c1a755 # v6
uses: docker/build-push-action@15560696de535e4014efeff63c48f16952e52dd1 # v6
Copy link
Member

Choose a reason for hiding this comment

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

Undo, updates to these versions are managed by dependabot.

with:
context: .
file: ./docker/Dockerfile
Expand Down
1 change: 1 addition & 0 deletions bandit/core/issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class Cwe:
IMPROPER_CHECK_OF_EXCEPT_COND = 703
INCORRECT_PERMISSION_ASSIGNMENT = 732
INAPPROPRIATE_ENCODING_FOR_OUTPUT_CONTEXT = 838
HARDCODED_SECRETS = 798

MITRE_URL_PATTERN = "https://cwe.mitre.org/data/definitions/%s.html"

Expand Down
97 changes: 97 additions & 0 deletions bandit/plugins/exposed_secrets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import functools
import re
import tomllib
from pathlib import Path

import bandit
from bandit.core import issue
from bandit.core import test_properties as test

_UNMATCHABLE_REGEX = re.compile(r"\b\B")

_config_filename = "secrets.toml"


@functools.cache
def _get_ignore_list(filename: str = _config_filename) -> list[re.Pattern]:
with Path(__file__).with_name(filename).open("rb") as f:
contents = tomllib.load(f)

# Compile the reject rules into regex patterns
return [re.compile(rule) for rule in contents.get("reject-rules", [])]


class _Secret:
id: str # unique identifier for the secret
description: str # description of the secret
regex: re.Pattern # compiled regex pattern for the secret
severity: str # severity level

def __init__(self, id: str, description: str, regex: str, severity: str):
self.id = id
self.description = description
self.regex = re.compile(regex) if regex else _UNMATCHABLE_REGEX
self.severity = severity


_GENERIC_SECRET = _Secret(
"generic", "Generic secret", regex="", severity="high"
)


def _make_issue(secret_spec: _Secret):
return bandit.Issue(
severity=bandit.HIGH, # severity is always high -> any leaked keys are critically bad
confidence=bandit.MEDIUM, # and confidence is always medium
cwe=issue.Cwe.HARDCODED_SECRETS,
text=f"{secret_spec.id} ({secret_spec.description}) secret is stored in a string.",
)


@functools.cache
def _get_database(filename: str = _config_filename) -> list[_Secret]:
with Path(__file__).with_name(filename).open("rb") as f:
contents = tomllib.load(f)

# contents is {'rules': [{'id': ..., 'description': ..., 'regex': ..., 'severity': ...}, ...]}
rules = contents.get("rules", [])
db = [
_Secret(
rule["id"], rule["description"], rule["regex"], rule["severity"]
)
for rule in rules
]

# remove all the secrets that are unmatchable
db = [secret for secret in db if secret.regex != _UNMATCHABLE_REGEX]
return db


def _is_ignored(string_to_check: str) -> bool:
# check if the string matches any ignore pattern
for pattern in _get_ignore_list():
if re.search(pattern, string_to_check):
return True
return False


def _detect_secrets(string_to_check: str) -> list[_Secret]:
res = []
db = _get_database()
for secret in db:
matches = re.search(secret.regex, string_to_check)
if matches is not None and not _is_ignored(matches.group()):
res.append(secret)
return res


@test.checks("Str")
Copy link
Member

Choose a reason for hiding this comment

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

A plugin that checks on every instance of an AST str does impact performance quite a bit. I realize we already have another plugin doing the same, but we should strive to avoid this as the speed of Bandit decreases signifiantly.

@test.test_id("B510")
Copy link
Member

Choose a reason for hiding this comment

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

B5xx is categorized as cryptography plugins. This plugin better fits in the misc group.

https://bandit.readthedocs.io/en/latest/plugins/index.html#plugin-id-groupings

def exposed_secrets(context):
Copy link
Member

Choose a reason for hiding this comment

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

Use the decorator @test.takes_config to signify that this plugin takes config, rather than implementing a new way to configure via the secrets.toml.

detected_secrets = _detect_secrets(context.string_val)
if len(detected_secrets) == 0:
return None
elif len(detected_secrets) == 1:
return _make_issue(detected_secrets[0])
else:
return _make_issue(_GENERIC_SECRET)
Loading