Skip to content

Commit d76f20e

Browse files
committed
fix: address top 10 audit findings across security, correctness, and reliability
- Add Lambda service pseudo-asset to graph so MAY_CREATE_LAMBDA edges are no longer silently dropped, restoring Lambda privesc detection - Add pagination to 6 network/ELB collector methods that were silently truncating data in accounts with >100 resources - Add non-root USER to Dockerfile.mcp to reduce container attack surface - Log import errors in cli/main.py instead of silently swallowing them - Validate --terraform-cmd against an allowlist of known binaries to prevent command injection via arbitrary binary paths - Pass IAM Deny statements to resource matching in access edge building to eliminate false-positive attack paths - Raise pydantic floor to >=2.6.3 to avoid CVE-2024-3772 (DoS via URL) - Treat missing/unresolvable nodes in attack paths as suspicious instead of legitimate; replace assert with proper None checks - Replace bare except Exception in IAM/network collectors with specific (ClientError, BotoCoreError) catches and log.warning - Remove debug print() and dead datetime.utcnow() call from scanner
1 parent b706ab6 commit d76f20e

10 files changed

Lines changed: 123 additions & 38 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ desktop.ini
8383
.claude/
8484
.cursor/
8585
.continue/
86+
CLAUDE.md
8687

8788
# Debug/temp files
8889
cyntrisec_v0.1.3_issue_reports

Dockerfile.mcp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates
55
WORKDIR /app
66
RUN git clone https://github.com/cyntrisec/cyntrisec-cli . && git checkout dd7cce0f6296da1280eb3b4770f361f08d05e555
77
RUN python -m pip install --break-system-packages .
8+
RUN useradd -m -s /bin/bash appuser
9+
USER appuser
810
CMD ["mcp-proxy","python","-m","cyntrisec","serve"]

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ dependencies = [
2929
"typer>=0.9.0",
3030
"rich>=13.0",
3131
"boto3>=1.34",
32-
"pydantic>=2.4",
32+
"pydantic>=2.6.3",
3333
"PyYAML>=6.0",
3434
"mcp>=1.0.0",
3535
]

src/cyntrisec/aws/collectors/iam.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22

33
from __future__ import annotations
44

5+
import logging
56
from typing import Any
67

78
import boto3
9+
from botocore.exceptions import BotoCoreError, ClientError
10+
11+
log = logging.getLogger(__name__)
812

913

1014
class IamCollector:
@@ -64,7 +68,13 @@ def _collect_inline_role_policies(self, role_name: str) -> list[dict]:
6468
RoleName=role_name,
6569
PolicyName=policy_name,
6670
)
67-
except Exception:
71+
except (ClientError, BotoCoreError) as e:
72+
log.warning(
73+
"Failed to get inline policy '%s' for role '%s': %s",
74+
policy_name,
75+
role_name,
76+
e,
77+
)
6878
continue
6979
policies.append(
7080
{
@@ -108,7 +118,8 @@ def _get_managed_policy_document(self, policy_arn: str) -> dict | None:
108118
if isinstance(doc, dict):
109119
return dict(doc)
110120
return {}
111-
except Exception:
121+
except (ClientError, BotoCoreError) as e:
122+
log.warning("Failed to get managed policy document for '%s': %s", policy_arn, e)
112123
return None
113124

114125
def _collect_instance_profiles(self) -> list[dict]:

src/cyntrisec/aws/collectors/network.py

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22

33
from __future__ import annotations
44

5+
import logging
56
from typing import Any
67

78
import boto3
9+
from botocore.exceptions import BotoCoreError, ClientError
10+
11+
log = logging.getLogger(__name__)
812

913

1014
class NetworkCollector:
@@ -28,14 +32,20 @@ def collect_all(self) -> dict[str, Any]:
2832
}
2933

3034
def _collect_vpcs(self) -> list[dict]:
31-
"""Collect VPCs."""
32-
response = self._ec2.describe_vpcs()
33-
return response.get("Vpcs", [])
35+
"""Collect VPCs with pagination."""
36+
vpcs = []
37+
paginator = self._ec2.get_paginator("describe_vpcs")
38+
for page in paginator.paginate():
39+
vpcs.extend(page.get("Vpcs", []))
40+
return vpcs
3441

3542
def _collect_subnets(self) -> list[dict]:
36-
"""Collect subnets."""
37-
response = self._ec2.describe_subnets()
38-
return response.get("Subnets", [])
43+
"""Collect subnets with pagination."""
44+
subnets = []
45+
paginator = self._ec2.get_paginator("describe_subnets")
46+
for page in paginator.paginate():
47+
subnets.extend(page.get("Subnets", []))
48+
return subnets
3949

4050
def _collect_security_groups(self) -> list[dict]:
4151
"""Collect security groups."""
@@ -46,25 +56,38 @@ def _collect_security_groups(self) -> list[dict]:
4656
return sgs
4757

4858
def _collect_route_tables(self) -> list[dict]:
49-
"""Collect route tables."""
50-
response = self._ec2.describe_route_tables()
51-
return response.get("RouteTables", [])
59+
"""Collect route tables with pagination."""
60+
tables = []
61+
paginator = self._ec2.get_paginator("describe_route_tables")
62+
for page in paginator.paginate():
63+
tables.extend(page.get("RouteTables", []))
64+
return tables
5265

5366
def _collect_internet_gateways(self) -> list[dict]:
54-
"""Collect internet gateways."""
55-
response = self._ec2.describe_internet_gateways()
56-
return response.get("InternetGateways", [])
67+
"""Collect internet gateways with pagination."""
68+
gateways = []
69+
paginator = self._ec2.get_paginator("describe_internet_gateways")
70+
for page in paginator.paginate():
71+
gateways.extend(page.get("InternetGateways", []))
72+
return gateways
5773

5874
def _collect_nat_gateways(self) -> list[dict]:
59-
"""Collect NAT gateways."""
60-
response = self._ec2.describe_nat_gateways()
61-
return response.get("NatGateways", [])
75+
"""Collect NAT gateways with pagination."""
76+
gateways = []
77+
paginator = self._ec2.get_paginator("describe_nat_gateways")
78+
for page in paginator.paginate():
79+
gateways.extend(page.get("NatGateways", []))
80+
return gateways
6281

6382
def _collect_load_balancers(self) -> list[dict]:
64-
"""Collect ELBv2 load balancers."""
83+
"""Collect ELBv2 load balancers with pagination."""
6584
try:
6685
elb = self._session.client("elbv2", region_name=self._region)
67-
response = elb.describe_load_balancers()
68-
return response.get("LoadBalancers", [])
69-
except Exception:
86+
lbs = []
87+
paginator = elb.get_paginator("describe_load_balancers")
88+
for page in paginator.paginate():
89+
lbs.extend(page.get("LoadBalancers", []))
90+
return lbs
91+
except (ClientError, BotoCoreError) as e:
92+
log.warning("Failed to collect load balancers in %s: %s", self._region, e)
7093
return []

src/cyntrisec/aws/relationship_builder.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -540,8 +540,9 @@ def build(self, assets: list[Asset]) -> list[Relationship]:
540540
Returns:
541541
List of new relationships to add
542542
"""
543-
# Ensure Internet asset exists
543+
# Ensure pseudo-assets exist
544544
self._ensure_internet_asset(assets)
545+
self._ensure_lambda_service_asset(assets)
545546

546547
# Build indexes
547548
self._index_assets(assets)
@@ -597,6 +598,23 @@ def _ensure_internet_asset(self, assets: list[Asset]) -> None:
597598
)
598599
assets.append(internet)
599600

601+
def _ensure_lambda_service_asset(self, assets: list[Asset]) -> None:
602+
"""Ensure the Lambda service pseudo-asset exists for MAY_CREATE_LAMBDA edges."""
603+
lambda_service_id = uuid.UUID("00000000-0000-0000-0000-00000000000a")
604+
for asset in assets:
605+
if asset.id == lambda_service_id:
606+
return
607+
608+
lambda_service = Asset(
609+
id=lambda_service_id,
610+
snapshot_id=self._snapshot_id,
611+
asset_type="pseudo:lambda-service",
612+
aws_resource_id="lambda-service",
613+
name="Lambda Service",
614+
properties={"description": "AWS Lambda service (target for MAY_CREATE_LAMBDA edges)"},
615+
)
616+
assets.append(lambda_service)
617+
600618
def _build_network_reachability_relationships(self) -> list[Relationship]:
601619
"""Build CAN_REACH edges based on network accessibility."""
602620
relationships: list[Relationship] = []
@@ -882,6 +900,7 @@ def _build_iam_access_relationships(self, assets: list[Asset]) -> list[Relations
882900
continue
883901

884902
policy_docs = role.properties.get("policy_documents", [])
903+
_, denied_resources = self._collect_policy_resources(policy_docs)
885904

886905
for target in sensitive_targets:
887906
target_arn = target.arn or target.aws_resource_id
@@ -901,9 +920,11 @@ def _build_iam_access_relationships(self, assets: list[Asset]) -> list[Relations
901920
if statement.get("Effect") != "Allow":
902921
continue
903922

904-
# Check if statement resources match target
923+
# Check if statement resources match target (considering denies)
905924
resources = self._normalize_resources(statement.get("Resource"))
906-
if not self._resources_match_target(resources, [], target_arn):
925+
if not self._resources_match_target(
926+
resources, denied_resources, target_arn
927+
):
907928
continue
908929

909930
# Get matched capabilities from this statement

src/cyntrisec/aws/scanner.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ def scan(
8989
Returns:
9090
Snapshot with scan results
9191
"""
92-
datetime.utcnow()
9392
start_time = time.monotonic()
9493

9594
try:
@@ -112,8 +111,7 @@ def scan(
112111
account_id = identity["Account"]
113112
log.info("Connected to AWS account: %s", account_id)
114113
except Exception as e:
115-
# Catch-all for credential/connection errors during init
116-
print(f"DEBUG: Caught exception in scanner: {type(e)} {e}")
114+
log.debug("Caught exception in scanner: %s %s", type(e).__name__, e)
117115
raise ConnectionError(f"Failed to authenticate with AWS: {e}") from e
118116

119117
# 2. Initialize storage

src/cyntrisec/cli/main.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,11 @@ def version():
105105
app.command("serve")(serve_cmd)
106106
app.command("remediate")(remediate_cmd)
107107
app.command("ask")(ask_cmd)
108-
except ImportError:
108+
except ImportError as _import_err:
109109
# Allow --version and --help to work even if deps missing
110-
pass
110+
logging.getLogger(__name__).warning(
111+
"Failed to register CLI commands (missing dependency?): %s", _import_err
112+
)
111113

112114

113115
if __name__ == "__main__":

src/cyntrisec/cli/remediate.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def remediate_cmd(
7474
terraform_cmd: str = typer.Option(
7575
"terraform",
7676
"--terraform-cmd",
77-
help="Terraform binary to invoke when using --execute-terraform",
77+
help="Terraform binary name (terraform, tofu, terragrunt)",
7878
),
7979
terraform_include_output: bool = typer.Option(
8080
False,
@@ -571,12 +571,37 @@ def _decode_bytes(value: object) -> str:
571571
return str(value)
572572

573573

574+
_ALLOWED_TERRAFORM_CMDS = {"terraform", "tofu", "terragrunt", "terraform.exe", "tofu.exe"}
575+
576+
577+
def _validate_terraform_cmd(terraform_cmd: str) -> str | None:
578+
"""Validate terraform_cmd is an allowed basename. Returns error message or None."""
579+
import os
580+
581+
basename = os.path.basename(terraform_cmd)
582+
if basename not in _ALLOWED_TERRAFORM_CMDS:
583+
return (
584+
f"terraform command '{terraform_cmd}' is not in the allowed list: "
585+
f"{', '.join(sorted(_ALLOWED_TERRAFORM_CMDS))}. "
586+
"Use a bare command name (e.g. 'terraform' or 'tofu')."
587+
)
588+
if terraform_cmd != basename:
589+
return (
590+
f"terraform command must be a bare name, not a path: '{terraform_cmd}'. "
591+
"Use 'terraform' or 'tofu' instead."
592+
)
593+
return None
594+
595+
574596
def _run_terraform(terraform_cmd: str, tf_dir: str, *, include_output: bool = False) -> dict:
575597
"""
576598
Run terraform apply -auto-approve against the generated hints.
577599
578600
Returns a dict with command and status. If terraform is missing, returns error.
579601
"""
602+
validation_err = _validate_terraform_cmd(terraform_cmd)
603+
if validation_err:
604+
return {"ok": False, "error": validation_err}
580605
if not shutil.which(terraform_cmd):
581606
return {"ok": False, "error": f"terraform command '{terraform_cmd}' not found"}
582607

@@ -614,6 +639,9 @@ def _run_terraform_plan(terraform_cmd: str, tf_dir: str, *, include_output: bool
614639
"""
615640
Run terraform plan (no apply) to validate generated module.
616641
"""
642+
validation_err = _validate_terraform_cmd(terraform_cmd)
643+
if validation_err:
644+
return {"ok": False, "error": validation_err, "exit_code": None}
617645
if not shutil.which(terraform_cmd):
618646
return {
619647
"ok": False,

src/cyntrisec/core/business_logic.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ def compute_delta(self, attack_paths: list[AttackPath]) -> list[AttackPath]:
7777

7878
def _is_entrypoint(self, asset: Asset) -> bool:
7979
"""Check if asset matches entrypoint criteria."""
80-
assert self.config is not None
80+
if self.config is None:
81+
return False
8182
criteria = self.config.entrypoints
8283

8384
# By ID
@@ -98,7 +99,8 @@ def _is_entrypoint(self, asset: Asset) -> bool:
9899

99100
def _matches_allowlist(self, asset: Asset) -> bool:
100101
"""Check if asset matches global allowlist tags."""
101-
assert self.config is not None
102+
if self.config is None:
103+
return False
102104
for tag_key, tag_pattern in self.config.global_allowlist.items():
103105
val = asset.tags.get(tag_key)
104106
if val and fnmatch.fnmatch(val, tag_pattern):
@@ -122,11 +124,8 @@ def _is_path_legitimate(self, path: AttackPath) -> bool:
122124
for asset_id in path.path_asset_ids:
123125
asset = self.graph.asset(asset_id)
124126
if not asset:
125-
continue
126-
127-
# If any node in the chain is NOT business-required, the path is suspect.
128-
# Exception: Maybe we allow traversal through unmarked nodes if the flow itself is marked?
129-
# That requires Critical Flow labeling (Edge labeling).
127+
# Missing/unresolvable nodes are suspicious, not legitimate
128+
return False
130129

131130
if self.LABEL_BUSINESS not in asset.labels:
132131
return False

0 commit comments

Comments
 (0)