[fix][push] check audience filter operators on the estimate endpoint - #7947
Open
ar2rsawseen wants to merge 1 commit into
Open
[fix][push] check audience filter operators on the estimate endpoint#7947ar2rsawseen wants to merge 1 commit into
ar2rsawseen wants to merge 1 commit into
Conversation
test, create and update all run the audience filter through validate(), which rejects unsafe Mongo operators in filter.user and filter.drill. estimate() builds the audience aggregation from the request directly, without validate(), so that check was skipped on this one endpoint. filter.user then reaches the $match stage of an aggregation over app_users unchecked, so a server-side-JS operator such as $function, including nested inside $expr, would execute in the database. Apply the same check estimate's siblings already use, before the aggregation runs. The check is the recursive operator walk, so it catches the operator at any depth. Legitimate audience filters, plain field predicates, are unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The push audience filter (
filter.user,filter.drill) is validated for unsafe Mongo operators insidevalidate(), whichtest,createandupdateall call.estimate()builds the audience aggregation from the request directly and never callsvalidate(), so that check was skipped on this one endpoint:So
filter.userreaches the$matchstage of an aggregation overapp_usersunchecked.Impact and grading
Measured against MongoDB 7.0:
$wheredoes not execute in an aggregation$match(server returnsBadValue), so the reported$wheredenial of service does not apply.$expr: {$function: {…}}does execute server-side JavaScript. That is the operator that matters, and the recursive operator check already blocks$functionat any depth, including nested inside$expr. It was simply never invoked here.$expr/$regexfield predicates run and can be used as a per-document boolean oracle, but they are legitimate query operators and are not blocked on any endpoint. They are not an escalation here: this endpoint requires push access on the app, and a member of an app can already read that app'sapp_usersthrough the membership-gated endpoints.Graded Low: an account with push access on the app is required, the operator that executes yields a denial of service rather than disclosure, and the disclosure oracle reaches only data the same member can already read. On a single-tenant deployment that is a trusted account degrading its own instance.
Change
Apply the same operator check the sibling handlers use, before the aggregation runs. countly-server uses
findUnsafeMongoOperator; countly-platform'svalidate()usesparseUserQuery, so itsestimatemirrors that. The check is recursive, so it catches the operator at any nesting depth.Scope
estimateis the only push audience path that skippedvalidate().test,createandupdatewere already guarded, verified in both repositories.Verification
{$match: filter}+{$count}sink on MongoDB 7.0:$whereerrors,$functionexecutes,$expr/$regexreturn counts.$functionat top level, nested in$expr, and four levels deep: blocked in every case.node --checkand lint clean on the changed server file; the platform change mirrors the existingvalidate()call in the same file and transpiles cleanly.