[fix][core] keep the stored member language a language code - #7932
Open
ar2rsawseen wants to merge 1 commit into
Open
[fix][core] keep the stored member language a language code#7932ar2rsawseen wants to merge 1 commit into
ar2rsawseen wants to merge 1 commit into
Conversation
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.
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 member language is stored by
/user/settings/langinfrontend/express/app.jswith no validation:That stored value then reaches file paths in around 25 readers.
api/utils/localization.jssanitizes it in bothgetPropertiesandgetProperty, but the two plugin localization lookups inplugins/plugins/api/api.jsconcatenate it as it comes:Change
Two independent layers:
localization.jsalready 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:
api/utils/localization.jsgetProperties,getPropertyplugins/plugins/api/api.jsplugin localization lookupplugins/plugins/api/api.jsplugins localization lookupplugins/reports/api/utils.jsgetLocaleLangStringlocale.getProperty, sanitizedplugins/reports/api/reports.jsmoment.locale(lang)api/parts/mgmt/mail.js,plugins/dashboards,plugins/star-ratingVerification
The accepted pattern against real and hostile values:
en,tr,de,zh,pt,pt-br,zh_CN,es../../../../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.