Skip to content

Commit 7f85dd3

Browse files
niksacdevclaude
andcommitted
security: fix critical code injection vulnerability
SECURITY FIX: Replace dangerous eval() usage with safe expression evaluator - Remove all eval() calls from orchestration modules (base.py, engine.py) - Create SafeConditionEvaluator for secure condition evaluation - Support all required operators: >, <, >=, <=, ==, !=, in, not in - Support compound conditions with 'and'/'or' logic - Prevent code injection attacks while maintaining functionality - Add comprehensive tests (10 test cases) including security isolation - All 38 tests passing (28 agent registry + 10 safe evaluator) Risk: eval() allowed arbitrary code execution via condition strings Fix: Whitelist-based expression parser with operator validation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 8e6ead9 commit 7f85dd3

File tree

5 files changed

+358
-25
lines changed

5 files changed

+358
-25
lines changed

loan_processing/agents/providers/openai/orchestration/base.py

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -219,15 +219,11 @@ def _validate_success_conditions(self, conditions: List[str], result: Dict[str,
219219
def _evaluate_condition(self, condition: str, result: Dict[str, Any]) -> bool:
220220
"""Evaluate a single condition against agent result."""
221221

222-
# Simple condition evaluation (can be enhanced with more sophisticated logic)
222+
# Safe condition evaluation without eval()
223223
try:
224-
# Replace field references with actual values
225-
for field, value in result.items():
226-
condition = condition.replace(field, repr(value))
227-
228-
# Evaluate the condition
229-
return eval(condition)
230-
except:
224+
from loan_processing.agents.shared.utils import evaluate_condition
225+
return evaluate_condition(condition, result)
226+
except Exception:
231227
return False
232228

233229

@@ -267,15 +263,11 @@ def check_handoff_conditions(
267263
def _evaluate_condition(self, condition: str, result: Dict[str, Any]) -> bool:
268264
"""Evaluate a single condition against agent result."""
269265

270-
# Simple condition evaluation (can be enhanced with more sophisticated logic)
266+
# Safe condition evaluation without eval()
271267
try:
272-
# Replace field references with actual values
273-
for field, value in result.items():
274-
condition = condition.replace(field, repr(value))
275-
276-
# Evaluate the condition
277-
return eval(condition)
278-
except:
268+
from loan_processing.agents.shared.utils import evaluate_condition
269+
return evaluate_condition(condition, result)
270+
except Exception:
279271
return False
280272

281273

loan_processing/agents/providers/openai/orchestration/engine.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,15 +229,11 @@ def _apply_decision_matrix(
229229
def _evaluate_condition(self, condition: str, result: Dict[str, Any]) -> bool:
230230
"""Evaluate a single condition against agent result."""
231231

232-
# Simple condition evaluation (can be enhanced with more sophisticated logic)
232+
# Safe condition evaluation without eval()
233233
try:
234-
# Replace field references with actual values
235-
for field, value in result.items():
236-
condition = condition.replace(field, repr(value))
237-
238-
# Evaluate the condition
239-
return eval(condition)
240-
except:
234+
from loan_processing.agents.shared.utils import evaluate_condition
235+
return evaluate_condition(condition, result)
236+
except Exception:
241237
return False
242238

243239
def _generate_error_decision(self, context: OrchestrationContext, error: Exception) -> LoanDecision:

loan_processing/agents/shared/utils/__init__.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@
88
from .config_loader import ConfigurationLoader
99
from .output_formatter import OutputFormatGenerator
1010
from .persona_loader import load_persona
11+
from .safe_evaluator import evaluate_condition, SafeConditionEvaluator
1112

1213
__all__ = [
1314
"ConfigurationLoader",
1415
"OutputFormatGenerator",
15-
"load_persona"
16+
"load_persona",
17+
"evaluate_condition",
18+
"SafeConditionEvaluator"
1619
]
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
"""
2+
Safe expression evaluator for condition checking.
3+
4+
This module provides secure alternatives to eval() for evaluating simple
5+
conditional expressions without the risk of code injection.
6+
"""
7+
8+
from __future__ import annotations
9+
10+
import re
11+
import operator
12+
from typing import Any, Dict
13+
14+
15+
class SafeConditionEvaluator:
16+
"""Safe evaluator for simple conditional expressions."""
17+
18+
# Allowed operators mapping
19+
OPERATORS = {
20+
'>': operator.gt,
21+
'<': operator.lt,
22+
'>=': operator.ge,
23+
'<=': operator.le,
24+
'==': operator.eq,
25+
'!=': operator.ne,
26+
'in': lambda a, b: a in b,
27+
'not in': lambda a, b: a not in b,
28+
}
29+
30+
@classmethod
31+
def evaluate_condition(cls, condition: str, context: Dict[str, Any]) -> bool:
32+
"""
33+
Safely evaluate a condition against a context.
34+
35+
Supports conditions like:
36+
- "credit_score > 650"
37+
- "status == 'approved'"
38+
- "risk_level in ['low', 'medium']"
39+
- "amount >= 1000 and amount <= 50000"
40+
41+
Args:
42+
condition: The condition string to evaluate
43+
context: Dictionary of field names to values
44+
45+
Returns:
46+
Boolean result of the condition evaluation
47+
48+
Raises:
49+
ValueError: If condition format is invalid or uses unsupported operators
50+
"""
51+
if not condition or not isinstance(condition, str):
52+
return False
53+
54+
condition = condition.strip()
55+
56+
# Handle compound conditions (and/or)
57+
if ' and ' in condition:
58+
parts = condition.split(' and ')
59+
return all(cls._evaluate_simple_condition(part.strip(), context) for part in parts)
60+
elif ' or ' in condition:
61+
parts = condition.split(' or ')
62+
return any(cls._evaluate_simple_condition(part.strip(), context) for part in parts)
63+
else:
64+
return cls._evaluate_simple_condition(condition, context)
65+
66+
@classmethod
67+
def _evaluate_simple_condition(cls, condition: str, context: Dict[str, Any]) -> bool:
68+
"""Evaluate a simple condition without compound operators."""
69+
70+
# Handle empty conditions
71+
if not condition or not condition.strip():
72+
return False
73+
74+
# Pattern to match: field_name operator value
75+
# Supports operators: >, <, >=, <=, ==, !=, in, not in
76+
pattern = r'^(\w+)\s*(>=|<=|==|!=|>|<|in|not\s+in)\s*(.+)$'
77+
match = re.match(pattern, condition)
78+
79+
if not match:
80+
raise ValueError(f"Invalid condition format: {condition}")
81+
82+
field_name, op, value_str = match.groups()
83+
84+
# Clean up operator
85+
op = op.strip()
86+
if op not in cls.OPERATORS:
87+
raise ValueError(f"Unsupported operator: {op}")
88+
89+
# Get field value from context
90+
if field_name not in context:
91+
raise ValueError(f"Field '{field_name}' not found in context")
92+
93+
field_value = context[field_name]
94+
95+
# Parse the comparison value
96+
comparison_value = cls._parse_value(value_str.strip())
97+
98+
# Perform the comparison
99+
try:
100+
return cls.OPERATORS[op](field_value, comparison_value)
101+
except (TypeError, ValueError) as e:
102+
raise ValueError(f"Cannot compare {field_value} {op} {comparison_value}: {e}")
103+
104+
@classmethod
105+
def _parse_value(cls, value_str: str) -> Any:
106+
"""Parse a value string into appropriate Python type."""
107+
108+
value_str = value_str.strip()
109+
110+
# Handle quoted strings
111+
if (value_str.startswith('"') and value_str.endswith('"')) or \
112+
(value_str.startswith("'") and value_str.endswith("'")):
113+
return value_str[1:-1] # Remove quotes
114+
115+
# Handle lists
116+
if value_str.startswith('[') and value_str.endswith(']'):
117+
# Simple list parsing - split by comma and parse each item
118+
items_str = value_str[1:-1] # Remove brackets
119+
if not items_str.strip():
120+
return []
121+
122+
items = []
123+
for item in items_str.split(','):
124+
items.append(cls._parse_value(item.strip()))
125+
return items
126+
127+
# Handle numbers
128+
try:
129+
# Try integer first
130+
if '.' not in value_str:
131+
return int(value_str)
132+
else:
133+
return float(value_str)
134+
except ValueError:
135+
pass
136+
137+
# Handle boolean
138+
if value_str.lower() == 'true':
139+
return True
140+
elif value_str.lower() == 'false':
141+
return False
142+
143+
# Handle None
144+
if value_str.lower() == 'none':
145+
return None
146+
147+
# Default to string
148+
return value_str
149+
150+
151+
def evaluate_condition(condition: str, context: Dict[str, Any]) -> bool:
152+
"""
153+
Convenience function for safe condition evaluation.
154+
155+
This is a drop-in replacement for dangerous eval() usage.
156+
157+
Example:
158+
>>> evaluate_condition("credit_score > 650", {"credit_score": 720})
159+
True
160+
>>> evaluate_condition("status == 'approved'", {"status": "pending"})
161+
False
162+
"""
163+
return SafeConditionEvaluator.evaluate_condition(condition, context)
164+
165+
166+
__all__ = ["SafeConditionEvaluator", "evaluate_condition"]

0 commit comments

Comments
 (0)