close
Skip to content

ext/lists: add max size check to genRange() to prevent OOM#1310

Open
marwan9696 wants to merge 3 commits into
google:masterfrom
marwan9696:fix-lists-range-dos
Open

ext/lists: add max size check to genRange() to prevent OOM#1310
marwan9696 wants to merge 3 commits into
google:masterfrom
marwan9696:fix-lists-range-dos

Conversation

@marwan9696
Copy link
Copy Markdown

lists.range(N) allocates N elements with no upper bound. A large
value like 2147483647 allocates ~16 GB and crashes the process.

This is dangerous in Kubernetes ValidatingAdmissionPolicy where
user-controlled input feeds into CEL expressions.

Add maxRangeSize (10M) check and reject negative values.

Fixes #1309

lists.range(N) allocates N elements with no upper bound. A large
value like 2147483647 allocates ~16 GB and crashes the process.

This is dangerous in Kubernetes ValidatingAdmissionPolicy where
user-controlled input feeds into CEL expressions.

Add maxRangeSize (10M) check and reject negative values.
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 30, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@marwan9696
Copy link
Copy Markdown
Author

CLA signed. Please re-check.

@TristonianJones
Copy link
Copy Markdown
Collaborator

/gcbrun

Comment thread ext/lists.go Outdated
}

// maxRangeSize is the maximum number of elements lists.range() will allocate.
const maxRangeSize = 10_000_000
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you make this a functional option for the Lists library instead?

Move the hardcoded maxRangeSize constant into a ListsMaxRangeSize
functional option on the Lists library, following the same pattern as
ListsVersion. Default remains 10,000,000. Setting to zero disables
the limit.
@marwan9696
Copy link
Copy Markdown
Author

Done — moved it to a ListsMaxRangeSize(n) functional option. Default is still 10M, setting to zero disables the check. Let me know if you'd like any other changes.

Comment thread ext/lists_test.go
}

// Over the limit should fail, not allocate.
_, err = genRange(defaultMaxRangeSize+1, defaultMaxRangeSize)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would you mind testing both the default range and a smaller range?

Comment thread ext/lists.go
return cel.Lib(l)
}

const defaultMaxRangeSize = 10_000_000
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We can make this 1M. Seems reasonably high that no one should ever hit it on accident.

@TristonianJones
Copy link
Copy Markdown
Collaborator

/gcbrun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ext/lists: lists.range() unbounded memory allocation allows OOM DoS

2 participants