Skip to content

Commit 5080546

Browse files
feat: Add nested comprehension depth limits (fixes #35)
Implements protection against resource exhaustion from deeply nested CEL comprehensions by enforcing a maximum nesting depth of 3 levels. **Changes:** - Added maxComprehensionDepth constant (value: 3) - Added comprehensionDepth field to converter struct to track nesting - Implemented depth checking in visitComprehension() with automatic increment/decrement using defer - Modified wrapConversionError() to preserve specific error messages through the wrapping chain - Added comprehensive test suite with 6 test functions covering all boundary conditions and edge cases **Security Impact:** - Prevents DoS attacks through expensive nested UNNEST/subquery operations - Addresses CWE-400: Uncontrolled Resource Consumption - Users attempting to exceed depth limit receive clear error messages **Examples:** - Depth 1-3: ✓ Allowed (e.g., list1.map(x, list2.filter(y, list3.exists(z, z > y)))) - Depth 4+: ✗ Rejected with error "comprehension nesting depth N exceeds maximum of 3" All existing tests continue to pass. Backward compatible for expressions within the depth limit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 3b23305 commit 5080546

File tree

4 files changed

+412
-28
lines changed

4 files changed

+412
-28
lines changed

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
# Changelog
22

3+
## [Unreleased]
4+
5+
### Security
6+
- **Nested Comprehension Depth Limits**: Added protection against resource exhaustion from deeply nested comprehensions (fixes #35)
7+
- Implemented maximum nesting depth of 3 for CEL comprehensions (all, exists, exists_one, filter, map)
8+
- Prevents DoS attacks through expensive nested UNNEST/subquery operations (CWE-400)
9+
- Clear error messages guide users to restructure overly complex queries
10+
- Example protected pattern: `list1.map(x, list2.filter(y, list3.exists(z, z > y)))` now limited to depth 3
11+
312
## [2.10.0] - 2025-10-10
413

514
### Added

cel2sql.go

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ const (
3939
// defaultMaxRecursionDepth is the default maximum recursion depth for visit()
4040
// to prevent stack overflow from deeply nested expressions (CWE-674: Uncontrolled Recursion).
4141
defaultMaxRecursionDepth = 100
42+
43+
// maxComprehensionDepth is the maximum nesting depth for CEL comprehensions
44+
// to prevent resource exhaustion from deeply nested UNNEST/subquery operations (CWE-400).
45+
maxComprehensionDepth = 3
4246
)
4347

4448
// ConvertOption is a functional option for configuring the Convert function.
@@ -267,16 +271,17 @@ func ConvertParameterized(ast *cel.Ast, opts ...ConvertOption) (*Result, error)
267271
}
268272

269273
type converter struct {
270-
str strings.Builder
271-
typeMap map[int64]*exprpb.Type
272-
schemas map[string]pg.Schema
273-
ctx context.Context
274-
logger *slog.Logger
275-
depth int // Current recursion depth
276-
maxDepth int // Maximum allowed recursion depth
277-
parameterize bool // Enable parameterized output
278-
parameters []interface{} // Collected parameters for parameterized queries
279-
paramCount int // Parameter counter for placeholders ($1, $2, etc.)
274+
str strings.Builder
275+
typeMap map[int64]*exprpb.Type
276+
schemas map[string]pg.Schema
277+
ctx context.Context
278+
logger *slog.Logger
279+
depth int // Current recursion depth
280+
maxDepth int // Maximum allowed recursion depth
281+
comprehensionDepth int // Current comprehension nesting depth
282+
parameterize bool // Enable parameterized output
283+
parameters []interface{} // Collected parameters for parameterized queries
284+
paramCount int // Parameter counter for placeholders ($1, $2, etc.)
280285
}
281286

282287
// checkContext checks if the context has been cancelled or expired.
@@ -1016,6 +1021,16 @@ func (con *converter) visitCallUnary(expr *exprpb.Expr) error {
10161021
}
10171022

10181023
func (con *converter) visitComprehension(expr *exprpb.Expr) error {
1024+
// Track comprehension nesting depth to prevent resource exhaustion (CWE-400)
1025+
con.comprehensionDepth++
1026+
defer func() { con.comprehensionDepth-- }()
1027+
1028+
// Check comprehension depth limit before context check (fail fast)
1029+
if con.comprehensionDepth > maxComprehensionDepth {
1030+
return fmt.Errorf("comprehension nesting depth %d exceeds maximum of %d",
1031+
con.comprehensionDepth, maxComprehensionDepth)
1032+
}
1033+
10191034
// Check for context cancellation before processing comprehensions (potentially expensive)
10201035
if err := con.checkContext(); err != nil {
10211036
return err

0 commit comments

Comments
 (0)