Skip to content

[fix][push] check audience filter operators on the estimate endpoint - #7947

Open
ar2rsawseen wants to merge 1 commit into
masterfrom
security/push-estimate-operator-guard
Open

[fix][push] check audience filter operators on the estimate endpoint#7947
ar2rsawseen wants to merge 1 commit into
masterfrom
security/push-estimate-operator-guard

Conversation

@ar2rsawseen

Copy link
Copy Markdown
Member

What

The push audience filter (filter.user, filter.drill) is validated for unsafe Mongo operators inside validate(), which test, create and update all call. estimate() builds the audience aggregation from the request directly and never calls validate(), so that check was skipped on this one endpoint:

// estimate(), no validate() call
const steps = await new Audience(log, new Message(data), app).steps({la: 1});
const cnt = await common.db.collection(`app_users${data.app}`)
    .aggregate(steps.concat([{$count: 'count'}])).toArray();

So filter.user reaches the $match stage of an aggregation over app_users unchecked.

Impact and grading

Measured against MongoDB 7.0:

  • $where does not execute in an aggregation $match (server returns BadValue), so the reported $where denial 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 $function at any depth, including nested inside $expr. It was simply never invoked here.
  • $expr / $regex field 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's app_users through 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's validate() uses parseUserQuery, so its estimate mirrors that. The check is recursive, so it catches the operator at any nesting depth.

Scope

estimate is the only push audience path that skipped validate(). test, create and update were already guarded, verified in both repositories.

Verification

  • Ran the four operator classes through the real {$match: filter} + {$count} sink on MongoDB 7.0: $where errors, $function executes, $expr/$regex return counts.
  • Ran the operator walk against $function at top level, nested in $expr, and four levels deep: blocked in every case.
  • node --check and lint clean on the changed server file; the platform change mirrors the existing validate() call in the same file and transpiles cleanly.

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.
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.

1 participant