Skip to content

[fix][core] keep the stored member language a language code (24.05) - #7933

Open
ar2rsawseen wants to merge 1 commit into
release.24.05from
backport/member-lang-path-validation-2405
Open

[fix][core] keep the stored member language a language code (24.05)#7933
ar2rsawseen wants to merge 1 commit into
release.24.05from
backport/member-lang-path-validation-2405

Conversation

@ar2rsawseen

Copy link
Copy Markdown
Member

Backport of #7932 to release.24.05.

What

The member language is stored by /user/settings/lang in frontend/express/app.js with no validation:

if (req.body.lang) {
    updatedUser.lang = req.body.lang;
    countlyDb.collection('members').update({"_id": ...}, {'$set': updatedUser}, ...);

That stored value then reaches file paths in around 25 readers. api/utils/localization.js sanitizes it in both getProperties and getProperty, but the two plugin localization lookups in plugins/plugins/api/api.js concatenate it as it comes:

local_path = fullpath + "/frontend/public/localization/" + resultObj.code + "_" + params.member.lang + ".properties";

Change

Two independent layers:

  1. Validate where it is stored, so every reader inherits it. A language code is accepted, anything else is refused and the request answers false as it already does for a missing value.
  2. Sanitize the two readers that build a path by hand, the same way localization.js already does.

A language code pattern rather than an allow list drawn from locale.conf.js, so a deployment shipping its own localization file keeps working, while leaving no way for path syntax to reach storage.

Why both layers

The value has around 25 readers across six files. Everything except these two goes through the sanitizing localization helpers or moment.locale, which is not a path. Fixing only the readers leaves the next reader exposed; fixing only the source leaves rows already stored. Doing both closes it now and keeps it closed.

Scope

Every reader of the stored language, with a verdict:

reader verdict
api/utils/localization.js getProperties, getProperty already sanitized
plugins/plugins/api/api.js plugin localization lookup sanitized here
plugins/plugins/api/api.js plugins localization lookup sanitized here
plugins/reports/api/utils.js getLocaleLangString goes through locale.getProperty, sanitized
plugins/reports/api/reports.js moment.locale(lang) not a path
api/parts/mgmt/mail.js, plugins/dashboards, plugins/star-rating all via the localization helpers

Verification

The accepted pattern against real and hostile values:

  • accepted: en, tr, de, zh, pt, pt-br, zh_CN, es
  • refused: ../../../../etc/passwd, .., /tmp/x, en/../../x, en%2f.., empty, ., an 80 character name, en;rm -rf /

Lint clean on both changed files in each repo. No behaviour change for the dashboard, which only sends codes from its own list.

The member language is stored from /user/settings/lang with no validation, and it reaches
file paths in several readers. api/utils/localization.js sanitizes it in both getProperties
and getProperty, but the two plugin localization lookups in plugins/plugins/api/api.js
concatenate it into a path as it comes:

    local_path = fullpath + "/frontend/public/localization/" + resultObj.code + "_" + params.member.lang + ".properties";

Validate the value where it is stored, so all readers inherit it, and sanitize the two
readers that build a path by hand, the same way localization.js already does. Both changes
are cheap and independent, which matters because the value has around 25 readers and only
these two had missed the helper.

Accepting a language code rather than an allow list from locale.conf.js keeps a deployment
that ships its own localization file working, while still leaving no way for path syntax to
reach storage.

No behaviour change for the dashboard, which only ever sends codes from its own list.
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