joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.77k stars 3.65k forks source link

[4.4.1] Language constants of some extensions not translated #42416

Closed toivo closed 11 months ago

toivo commented 11 months ago

Steps to reproduce the issue

Download the Helix Ultimate v2.0.17 plugin and template from https://www.joomshaper.com/downloads/template/helixultimate Install these extension in Joomla 4.4.0. Update the website to Joomla 4.4.1.

Expected result

The language constants are translated.

Actual result

The language constants are not translated.

System information (as much as possible)

N/A

Additional comments

Reported in Joomla forum topics https://forum.joomla.org/viewtopic.php?f=815&t=1005723 and https://forum.joomla.org/viewtopic.php?f=815&t=1005719.

The same happened to my own backend component under development in Joomla 4.4.1 and 5.0.1. Only the language constants used in the manifest file and defined in the mycomponent.ini file got translated.

Revert the two files LanguageHelper.php and Text.php in the folder libraries/src/Language to the previous versions to fix the translation issue.

Question: Do the changes in the Language folder change the way the items in the manifest file have to be presented or something else? The change in 4.4.1 and 5.0.1 will no doubt break many third party extensions, even if they have followed the Joomla documentation to the letter, unless the documentaion is updated or some changes deprecated or reverted.

shur commented 11 months ago

Confirmed, same problem with all modules by RAXO https://extensions.joomla.org/extension/news-display/articles-display/raxo-all-mode-pro/

After updating to Joomla 4.4.1, all language constants in the module control panel are gone. Users are already starting to pour into the support team about this issue. So the problem is present and this is not an isolated case.

brianteeman commented 11 months ago

I have replicated the issue with the helix template and can confirm that with the security changes the issue is present. However the root cause of the issue is the language files themself. Both the site and the admin language files for the template are invalid.

Both of them have a faulty string for COM_FINDER_ADVANCED_TIPS

image

As can be seen there is one invalid string. Instead of it being on one line ending with " it is spread across multiple lines.

As soon as you remove the hard returns in the string everything works.

This is not a core bug ut a problem in the language file that is now exposed.

brianteeman commented 11 months ago

@HLeithner @SniperSister Please confirm my findings.

Note: 5.0.0 with the language debug did not report an error in the language file but 5.0.1 does

shur commented 11 months ago

@brianteeman

As can be seen there is one invalid string. Instead of it being on one line ending with " it is spread across multiple lines. As soon as you remove the hard returns in the string everything works.

Yes, the problem is also in one language constant, which contains a long description of the module and therefore takes up several lines. As soon as you remove the line breaks the problem disappears.

Brian, thank you for your quick and accurate response.

SniperSister commented 11 months ago

The problem is indeed related to the security-fix. We had to change the parsing mode of PHP's ini methods to fix a security vector. The NORMAL mode used so far allowed linebreaks within multiline i18n strings, where the now used RAW mode does not. We tested the change on various testing and production sites before release, however the specific issue was overlooked as none of the test sites used language files with linebreaks within strings and the documentation of the method on php.net does not mention that behavior.

Reverting the mode change is not an option as the attack vector would be re-opened again, so the proper way forward is indeed to update the line breaks.

brianteeman commented 11 months ago

Thanks for confirming my findings.

Maybe you should document this

shur commented 11 months ago

I'll just leave this thought here. Is there any way to make it so that if there is a multiline problem in one language constant, the whole localization for the extension doesn't fail? So that the problem in one constant remains a problem in one constant.

SniperSister commented 11 months ago

Both PHP core methods, parse_ini_file and parse_ini_string, consume the complete i18n file as an input and either fail or succeed completely. There is no option to skip a single faulty string.

toivo commented 11 months ago

No problem. The JED Checker extension chould be updated to report linebreaks and warn to extension developers. I will create a feature request asap.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42416.

toivo commented 11 months ago

JED Checker feature request: JED Checker to report linebreaks in language strings


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42416.

BrainforgeUK commented 11 months ago

Changing the JED checker does not solve the problem!

In the meantime v4.4.1 is not useable!

Added \ to escape the new line does not appear to work. What is the workaround for this?

Removing the newlines from a large .ini file is not acceptable. Does my extension have to load the language file by itself?

Could we add an option to the extension manifest option to turn raw mode off? Quick fix undo the change for 4.4.2 and then add this new manifest option for 4.4.3. Will that satisfy the security issue?

SniperSister commented 11 months ago

What is the workaround for this?

Removing the linebreak is the workaround.

Could we add an option to the extension manifest option to turn raw mode off?

No because, disabling the RAW mode for any installed extensions would re-introduce the vulnerability.

brianteeman commented 11 months ago

it will take you less time to fix your language file than it took you to write your post

brianteeman commented 11 months ago

Note that the documentation for language files states

Each line consists of a key-value pair separated by an equals sign https://docs.joomla.org/Creating_a_language_definition_file

BrainforgeUK commented 11 months ago

How to add a multi-line translateable hint for a textarea form field?

SniperSister commented 11 months ago

That's a interesting usecase @BrainforgeUK! Are you using the hint in context of a JForm XML or is it a manually rendered HTML tag?

brianteeman commented 11 months ago

This works as a multiline hint in a text area JMULTILINE="Line1\nLine2\nLine3"

image

BrainforgeUK commented 11 months ago

JForm I use Joomla standard form types everywhere - or derivatives of. https://docs.joomla.org/Standard_form_field_types

BrainforgeUK commented 11 months ago

So what is this security vector this change is fixing?

Would it not be better to replace parse_ini_file() with custom Joomla code in the language helper file? At least the logic would then be in the Joomla world when dealing with any future issues.

A quick trawl of parse_ini_file() security issues (see below) mention some hosts disallow it. How widespread is that? Another reason to get stop using parse_ini_file()?

Banning newlines in language strings makes maintaining .ini language files with long descriptive text very difficult. Consider the long strings one can put together, with embedded HTML, when using the note form field type. https://docs.joomla.org/Note_form_field_type Ok - one could put such text in the form XML using CDATA, but then that throws translatable notes out of the window. Or even extend the note class ... but that is dealing with a language translation problem which should not be there.

https://stackoverflow.com/questions/35067087/is-the-php-function-parse-ini-file-really-so-dangerous

https://bugs.php.net/bug.php?id=34949

https://github.com/dweeves/magmi-git/issues/349

https://www.webhostingtalk.com/showthread.php?t=867864

brianteeman commented 11 months ago

Did you actually try the string I suggested? Worked perfectly for me as shown in the screenshot. If it didnt work for you please let me know

kulbabskyy commented 11 months ago

i have the same issue in the PayPlans component. i have removed all \n strings but with no result

BrainforgeUK commented 11 months ago

Did you actually try the string I suggested? Worked perfectly for me as shown in the screenshot. If it didnt work for you please let me know

... Thanks \n worked for the textarea I looked at - its the other long descriptive text with embedded HTML. Stringing it all together without linebreaks makes maintenance difficult ... linewraps in the IDE helps but the text is still a big jumble.

brianteeman commented 11 months ago

if its embedded html then in that case you can use <br> as we do in core already

brianteeman commented 11 months ago

@kulbabskyy

The only multiline string I can see in payplans is

COM_PAYPLANS_PLAN_GRID_CAN_NOT_DELETE_PLAN_SUBSCRIPTION_EXISTS="Deletion of a plan is possible only when you delete all corresponding subscriptions. Till then, you can only disable the selected plan. <br> <br>
If you are still unable to delete a plan then make sure you have set <strong>Delete Incomplete Orders</strong> under <strong>Backend Settings >> System</strong>, so this will delete the incomplete order on the cron job as per specified time set in the setting."

Change it to

COM_PAYPLANS_PLAN_GRID_CAN_NOT_DELETE_PLAN_SUBSCRIPTION_EXISTS="Deletion of a plan is possible only when you delete all corresponding subscriptions. Till then, you can only disable the selected plan. <br> <br>If you are still unable to delete a plan then make sure you have set <strong>Delete Incomplete Orders</strong> under <strong>Backend Settings >> System</strong>, so this will delete the incomplete order on the cron job as per specified time set in the setting."
kulbabskyy commented 11 months ago

@brianteeman Thank you, it solved my problem! i have submited your solution to the PayPlans developers

BrainforgeUK commented 11 months ago

if its embedded html then in that case you can use <br> as we do in core already

Thanks, that does not solve my issue. In a small way the COM_PAYPLANS_PLAN_GRID_CAN_NOT_DELETE_PLAN_SUBSCRIPTION_EXISTS example illustrates the problm. Removing the newline works but having one long unformatted string to maintain is what I find unacceptable. The newlines are not apparent on the website but easier to maintain in the IDE.

I see 2 possible solutions...

(a) Keep the newlines in the source - as seen in the IDE / stored on github. Modify the tool I use which packages my extensions into zip files for distribution so it removes the newlines.

(b) Modify the Joomla language code so instead of calling parse_ini_file() it does this ... Read the file into a string Remove the embedded newlines (no other parsing, content changes) _Call parse_inistring() Generic solution for everybody.

Having the vision for (b) when I have the time I can submit a pull request for that and it can be argued over separately. The JED rules would need to be changed back to allowing newlines.

bembelimen commented 11 months ago

Pre-Parse every language file which is loaded will for sure have a performance impact.

(c) set up your IDE, that it auto wraps long lines (ALT + Z in Visual Studio Code or View => Word Wrap)

ghost commented 11 months ago

Thanks, Joomla, for giving me a very hearty laugh this week.

I can't believe someone decided making "sensible information" [sic] available is an issue. There is no way to exploit the LanguageHelper::parseIniFile() function as it requires a resource on the filesystem, unless Joomla is now claiming that every language file is untrusted (and if the language files are untrusted, why does Joomla allow including and executing untrusted PHP files, or installing extensions which can add untrusted resources to my server?). And somehow, the "contributors" on this repository have spun this around to claim every extension developer who wrote a language file that PHP accepts without error is wrong and forced to waste time making changes to their extensions because someone decided they don't want an extension developer to be able to use environment variables in their language files. Great job, team.

Does this mean Joomla considers any use of environment variables a security risk? If so, then why isn't $_ENV reset to an empty state when Joomla is launched, and why does Joomla use the getenv() function? And what about $_SERVER which has a lot more "sensible information" available, does that need to be blocked too?

SniperSister commented 11 months ago

Thanks, Joomla, for giving me a very hearty laugh this week.

Happy to hear! :) Have a great weekend!

jschmi102 commented 11 months ago

Great! Because of this problem, developers are motivated to work: checking, debugging, changing code of extensions, asking users for patience ....

BrainforgeUK commented 11 months ago

Hearty laugh comment - I did wonder if this was all a waste of time and maybe even related to issue 42416 and rumours of hosts which for security issues block parse_ini_file() e.g. the links I added to an earlier comment.

Anyway I have put in a pull request which I hope might keep everyone happy (me in particular)!

42441

BrainforgeUK commented 11 months ago

Just been informed of this issue on a very old extension of mine someone is using on Joomla 4 and upgraded to 4.1.1

Told them to use language overrides, I don't want to look at that old code ... don't know what it might uncover. If it works on J4 just keep quiet ... J5/6 rediness?

Anyway it raises the point that unless this is fixed (such as my pull request or reverting to J4.1.0 behaviour) this issue will keep rearing its head for sometime to come - and from the initial comments it won't just be issues related my extensions.

BrainforgeUK commented 11 months ago

Looks like Joomla documentation is wrong. Also there is no need for Joomla language handling to use parse_ini_file(). See pull request #42441

alikon commented 11 months ago

closed in favour of #42441