Skip to content

[fix][core] restrict export projections to include and exclude (24.05) - #7946

Open
ar2rsawseen wants to merge 1 commit into
release.24.05from
backport/export-projection-sanitize-2405
Open

[fix][core] restrict export projections to include and exclude (24.05)#7946
ar2rsawseen wants to merge 1 commit into
release.24.05from
backport/export-projection-sanitize-2405

Conversation

@ar2rsawseen

Copy link
Copy Markdown
Member

Backport of #7945 to release.24.05.

What

/o/export/db parses the caller's projection and passes it to find() unchanged:

var cursor = options.db.collection(options.collection).find(options.query, {"projection": options.projection});

MongoDB 4.4 and later evaluate aggregation expressions in a find projection, so a value can rename or compute a field rather than select one. The credential redaction that runs on the cursor removes fields by name, so a renamed field carries its value into the export file under a name the redaction does not know about.

Measured against MongoDB 7.0.37, all of these are accepted by find() and return the underlying values:

projection accepted
{"pw": "$password", "ak": "$api_key", "tfa": "$two_factor_auth"} yes, renames past the redaction
{"secret": "$two_factor_auth.secret_token"} yes, nested paths too
{"both": {"$concat": ["$password", "$api_key"]}} yes, computed values
{"x": {"$function": {"body": "...", "lang": "js"}}} yes, evaluates javascript in the database engine

The DB Viewer already guarded its own projections for this exact reason, and its comment names the vector, including the $function case. The guard was never applied to the export path, although the commit that added the export redaction set out to align the two.

Change

  • common.sanitizeProjection in api/utils/common.js: one implementation, restricting a projection to 0, 1, true and false.
  • plugins/dbviewer/api/parts/query_guard.js delegates to it, so the DB Viewer and the export cannot drift apart again.
  • The export handler applies it after the projection is parsed.

Scope

Every place a caller influenced projection can reach a database, across the three repositories:

site verdict
/o/export/db -> exports.fromDatabase -> find(query, {projection}) fixed here, both this repo and countly-platform
DB Viewer find path already guarded, now via the shared helper
/o/export/request -> exports.fromRequest not a database projection: the object is only used to order columns in the output
plugins/views $project pipelines built from internal level names, not from the request
plugins/surveys aggregation options.project no caller sets it, in either repo
countly-platform DB Viewer ClickHouse path different engine, no aggregation expressions in play. Flagged separately, see below

Not changed, on purpose

  • sort and formatFields on the same handler are also parsed from the request. A sort value is not an expression context in the same way, and the dashboard sends real sort objects, so tightening them here would risk breaking exports for no security gain.
  • The ClickHouse projection in countly-platform's DB Viewer (plugins/dbviewer/api/api.js) takes a caller supplied object into a ClickHouse query builder. Mongo style expressions do not apply there, so this guard is not the right tool, and whether that builder is safe with arbitrary keys is a separate question worth its own look rather than a change made in passing.

Verification

  • test/unit-tests/api.utils.common.js gains 6 cases: plain include and exclude survive untouched, field path aliases and nested aliases are dropped, $concat, $function and $cond are dropped, invalid values such as 2, NaN and strings are dropped, and a missing or non object projection is handled. Replacing the guard with a pass through fails 4 of them.
  • The existing 7 DB Viewer query guard cases still pass through the delegation.
  • On release.24.05 three unrelated cases in that file already fail on stock (mongodb.ObjectID is not a function, a driver artifact). The 6 added cases pass there.

/o/export/db parses the caller's projection and hands it to find() as given. MongoDB 4.4
and later evaluate aggregation expressions in a find projection, so a value like
{"pw": "$password"} renames a field rather than selecting one, and the credential redaction
that runs afterwards removes fields by name. A renamed field therefore carries its value
into the export under a name the redaction does not know. The same applies to computed
expressions, and $function evaluates javascript inside the database engine.

The DB Viewer already guarded its own projections for exactly this reason, and its comment
names the vector. That guard was never applied to the export path, even though the commit
that added the export redaction set out to align the two.

Move the guard into api/utils/common.js so there is one implementation, have the DB Viewer
helper delegate to it, and apply it to the export path after the projection is parsed.

Plain include and exclude are untouched, which is everything the dashboard sends: the
datatable exports pass field lists of 0 and 1.
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