Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Article can't be saved/updated by the user after 3.7.0 updation #15673

Closed
kozhevn opened this issue Apr 28, 2017 · 56 comments
Closed

Article can't be saved/updated by the user after 3.7.0 updation #15673

kozhevn opened this issue Apr 28, 2017 · 56 comments
Assignees

Comments

@kozhevn
Copy link

kozhevn commented Apr 28, 2017

Steps to reproduce the issue

Backend:

  1. Create user with only Registered+Author+Editor+Publisher access rights.
  2. Create an article, set this user as author

Frontend:
3. Logged in with this user account
4. Edit article (editor doesn't matter), put simple content <h3> test <\h3>
5. As can see 'Save' button works fine, article content updated.
6. Edit again and put the text from attachment content.txt or something another with few html tags.
7. 'Save' button produced timeout error and article updation doesn't happens.

Restoring the file 'libraries/joomla/filter/input.php' to version < 3.7.0 fixes the issue.

Expected result

At the position 7 'Save' button produce fast content updation without error.

Actual result

'Save' button produce timeout error over a time and article updation doesn't happens.

System information (as much as possible)

Linux 4.4.0-67-generic #88-Ubuntu SMP Wed Mar 8 16:34:45 UTC 2017 x86_64
Ubuntu 16.04.2 LTS
PHP 7.0.15-0ubuntu0.16.04.4
Joomla! 3.7.0 Stable [ Amani ] 25-April-2017 15:36 GMT
Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT

Additional comments

Behaviour looks like posted in #15628 but issue is another in the reproduction ( can be reproducted only for user and doesn't requred a big content, for me it was happens even with one <p> tag content from example ).

Restoring only the 'libraries/joomla/filter/input.php' to version < 3.7.0 fixes the issue.

Issue happens in frontend only (created user have no backend access).

When superuser edit the article under backend, all works just fine.

It seems looks like as an infinity loop during article content filtration functionality by the 'libraries/joomla/filter/input.php'.
Probably the filtration doesn't happens for superuser under backend.

@kozhevn kozhevn changed the title Article can't be saved by user after 3.7.0 updation Article can't be saved/updated by user after 3.7.0 updation Apr 28, 2017
@kozhevn kozhevn changed the title Article can't be saved/updated by user after 3.7.0 updation Article can't be saved/updated by the user after 3.7.0 updation Apr 28, 2017
@kozhevn kozhevn changed the title Article can't be saved/updated by the user after 3.7.0 updation Article can't be saved/updated by user after 3.7.0 updation Apr 28, 2017
@ameshkoff
Copy link

ameshkoff commented Apr 28, 2017

I have a similar issue even as a superuser in the backend adding some "difficult" tags:

  • <a> tag with rel attribute (added by TinyMCE by default). Without rel <a> works fine
  • <hr id="system-readmore" /> tag

Timeout error.

@csthomas
Copy link
Contributor

What php version?

@ameshkoff
Copy link

PHP 5.6.29

@ameshkoff
Copy link

And yes, downgrading libraries/joomla/filter/input.php saves the day.

@csthomas
Copy link
Contributor

Call Stack

Time Memory Function Location

1 0.0001 382696 {main}( ) .../index.php:0
2 0.0130 905520 JApplicationCms->execute( ) .../index.php:49
3 0.0130 905520 JApplicationSite->doExecute( ) .../cms.php:265
4 0.0304 1742024 JApplicationSite->dispatch( ) .../site.php:230
5 0.0385 1847752 JComponentHelper::renderComponent( ) .../site.php:191
6 0.0388 1862456 JComponentHelper::executeComponent( ) .../helper.php:369
7 0.0389 1879464 require_once( '.../components/com_content/content.php' ) .../helper.php:394
8 0.0395 1899368 JControllerLegacy->execute( ) .../content.php:39
9 0.0395 1899368 ContentControllerArticle->save( ) .../legacy.php:709
10 0.0395 1899368 JControllerForm->save( ) .../article.php:289
11 0.0555 2408224 ContentModelArticle->validate( ) .../form.php:703
12 0.0561 2408224 JModelForm->validate( ) .../article.php:481
13 0.0561 2408248 JForm->filter( ) .../form.php:350
14 0.0568 2437184 JForm->filterField( ) .../form.php:229
15 0.0568 2437296 JComponentHelper::filterText( ) .../form.php:1527
16 0.0571 2445048 JFilterInput->clean( ) .../helper.php:291
17 0.0571 2445080 JFilterInput->remove( ) .../input.php:344
18 299.9985 2510616 JFilterInput->_cleanTags( ) .../input.php:790
19 299.9985 2510616 JFilterInput->cleanTags( ) .../input.php:809
20 300.1033 2679536 Joomla\String\StringHelper::substr( ) .../input.php:1033
21 300.1033 2679536 utf8_substr( ) .../StringHelper.php:223
22 300.1033 2679536 mb_substr ( ) .../core.php:94

@PhilETaylor

This comment was marked as abuse.

@csthomas
Copy link
Contributor

csthomas commented Apr 29, 2017

This should help someone to write a patch:)

Example 1:

<ul>
<li><a href="../">презентация</a>)</li>
<li>Елфимова О.Т. Разработка системы отделения космического аппарата Метеор-М в системе MSC.Adams<a style="color: maroon;" href="../../pub/diplom_labors/2016/2016_Elfimova_O_rpz.pdf">диплом</a></li>
</ul>

which does not works.

Example 2:

preg_match_all('/[aäeëioöuáéíóú]/u', 'hëllo', $out, PREG_OFFSET_CAPTURE);print_r($out);
Array
(
    [0] => Array
        (
            [0] => Array
                (
                    [0] => ë
                    [1] => 1
                )

            [1] => Array
                (
                    [0] => o
                    [1] => 5 // should be 4 but PREG_OFFSET_CAPTURE returns OFFSET IN BYTES
                )
        )
)

Links:
http://stackoverflow.com/questions/2187615/utf-8-characters-in-preg-match-all-php
http://stackoverflow.com/questions/1725227/preg-match-and-utf-8-in-php

@joomla-cms-bot joomla-cms-bot changed the title Article can't be saved/updated by user after 3.7.0 updation Article can't be saved/updated by the user after 3.7.0 updation Apr 29, 2017
@AlexRed
Copy link
Contributor

AlexRed commented Apr 29, 2017

I can confirm:
error-rel

@n3t
Copy link
Contributor

n3t commented Apr 29, 2017

Confirm: StringHelper functions in JFilterInput causes infinite loop for UTF8 content.
PHP 7.0.14, iconv and mbstring installed.

This is is really big issue, as it breaks basic Joomla functions.


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

@n3t
Copy link
Contributor

n3t commented Apr 29, 2017

Issue occours even in backend, if logged in user is in usergroup with Text Filters set to Default Blacklist.


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

@infograf768
Copy link
Member

infograf768 commented Apr 30, 2017

I do not get the timeout error (php 5.4.4), but I do get the undefined created_by and modified_by Notices

@Bilal-Abdeen
Copy link

Bilal-Abdeen commented May 1, 2017

I get the problem with the following configuration:

  • PHP: 7.1.3
  • Database Version: 5.5.51-38.2
  • Database Collation: utf8_unicode_ci
  • Database Connection Collation: utf8mb4_general_ci
  • Joomla: 3.7.0 Stable [ Amani ] 25-April-2017 15:36 GMT
  • Editor: JCE
  • NON-administrator User <<<---
  • Text having Unicode and **<a>** element <<<---

Error Message:
Fatal error: Maximum execution time of 30 seconds exceeded in /libraries/vendor/joomla/string/src/phputf8/mbstring/core.php on line 94

@hacki65
Copy link
Contributor

hacki65 commented May 1, 2017

Can not confirm.
Tested with new installation AND updated site.

  • dbversion: 5.5.55-0ubuntu0.14.04.1
  • dbcollation: utf8_general_ci
  • dbconnectioncollation: utf8mb4_general_ci
  • phpversion: 7.0.18
  • server: Apache
  • sapi_name: fpm-fcgi
  • version: Joomla! 3.7.0 Stable [ Amani ] 25-April-2017 15:36 GMT
  • Editor: TinyMCE

Have everyone who has the issue saved the plugin "Editor - TinyMCE" because of the warning message, that TinyMCE has been updated?

@Bilal-Abdeen
Copy link

Did you try it with a (1) "non-administrator user", (2) used UNICODE text (not only English), and (3) the text has a hyperlink?

@n3t
Copy link
Contributor

n3t commented May 1, 2017

In my case it occured when using JCE. The issue is highly dependent on article content.
However I tried fresh Joomla installation and here are exact setps to reproduce (PHP 5.6.20, MySQL 5.5.5-10.0.23-MariaDB)

  1. Fresh Joomla 3.7.0 Installation, no demo data, no extra language
  2. Login to administrator
  3. Create new user, assign him to user group Manager
  4. Logout and login as newly created user
  5. Create new article, give it any name
  6. Click Toggle editor and paste the content bellow
  7. Save
<p style="text-align: justify;"><strong>Nafta nebo baterie? Za nás jednoznačně to druhé. Před pár dny jsme si vyzvedli nový elektromobil. Nyní jej testujeme a zatím můžeme říct jedno - pozor, toto vozítko je vysoce návykové!</strong></p>
<hr id="system-readmore" />
<p style="text-align: justify;"><a href="http://www.example.com" target="_blank" rel="noopener noreferrer">Auta.</a> </p>

This will lead to PHP timeout.

@hacki65
Copy link
Contributor

hacki65 commented May 2, 2017

@Bilal-Abdeen : Yep, steps reproduced like initial issue post with the content of the textfile "content.txt" and with non-admin user.

With the steps reproduced from n3t i can confirm this behaviour. If textfilter for group Manager is set to no filtering there is no timeout error.

So it seems to be an issue with the filtering.

@Bilal-Abdeen
Copy link

@hacki65,
Thank you very much. Removing filtering (Global Configuration-> text Filters -> Select Uses' Group) seems to stop the error. I think this a good temporary workaround until a code fix is found. Obviously, this temporary fix can be a security issue and should be reversed as soon as a real fix is implemented.

@andpuxa
Copy link

andpuxa commented May 2, 2017

When I save the material typed by the Cyrillic user under the username of the super user, at the end add a lot of characters >> """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" <<
Restoring 'libraries/joomla/filter/input.php' to version 3.6.5 fixes the issue.

@infograf768
Copy link
Member

Concerning the Notices (only) please test #15753

@quercusdk
Copy link

quercusdk commented May 3, 2017

  • Joomla 3.7.0
  • PHP 7.0.10
  • Apache/2.4.23 (Ubuntu 16.04)
  • JCE Editor Pro

Can confirm that my custom plugin using JCE Editor to save large HTML-text reach "Maximum execution time of 120 seconds" when trying to save and update from back-end as Super User.
Restoring 'libraries/joomla/filter/input.php' to version 3.6.5 fixes the issue.

@csthomas
Copy link
Contributor

csthomas commented May 4, 2017

Does anybody work on that?
I probably does not have enough time but idea could be:

  • revert at least changes (StringHelper::) in methods which use preg_match(.. PREG_OFFSET_CAPTURE);
  • add to above regex /u modifier - this not solve the problem
  • instead substr with offset use native preg_match(.. PREG_OFFSET_CAPTURE, $offset);

@SniperSister
Copy link
Contributor

I didn't had the time to dive into this, but a quick feedback on the possible ideas by @csthomas

revert at least changes (StringHelper::) in methods which use preg_match(..PREG_OFFSET_CAPTURE);

That's not an option ;) the changes have been made for security reasons, we need proper (=multibyte-aware) code here

add to above regex /u modifier

Sounds like the best option to me

instead substr with offset use native preg_match(.. PREG_OFFSET_CAPTURE, $offset);

This would only work with the /u modifer again I guess, because eitherwise we'll run into the same mb-offset issues that we had in the old code

@csthomas
Copy link
Contributor

csthomas commented May 4, 2017

All offsets from preg_... with PREG_OFFSET_CAPTURE are offset of bytes.
For now joomla juggles between single bytes and multi-bytes offset.

preg gives single byte offset but StringHelper::.. think it is multibyte offset - and there is a problem.

@csthomas
Copy link
Contributor

csthomas commented May 9, 2017

There is a nice html validator http://www.bioinformatics.org/phplabware/internal_utilities/htmLawed/
which does not use any mb_.. functions.

@PhilETaylor

This comment was marked as abuse.

@mbabker
Copy link
Contributor

mbabker commented May 9, 2017

JFilterInput isn't just a validator (actually I don't even know if I'd call that one of its purposes). It's an input filtering system and any good system must be multibyte aware.

@photodude
Copy link
Contributor

@PhilETaylor it looks like it uses some bytecode in preg_* functions to be multibyte aware.... but see @mbabker's comment.

@PhilETaylor

This comment was marked as abuse.

@csthomas
Copy link
Contributor

csthomas commented May 9, 2017

I put it as an example. It is not appropriate time for such changes.

preg_replace/match and all regex functions can be multibyte aware but return offset (PREG_OFFSET_CAPTURE) as offset in bytes.

IMO If joomla should use the functions mb_ or utf8_ then regex functions with flag PREG_OFFSET_CAPTURE do not fit.

@photodude
Copy link
Contributor

I've added some test cases with Greek and one with Russian characters #15914
As you can see they work just fine in the base cases. The other example case using Russian characters #15810 is having issues during the removal of the tags.

@photodude
Copy link
Contributor

I've created another PR in the framework using Persian characters following some of the "Fred" cases which also cause the filter to fail, but do not cause the recursion timeout issue.
joomla-framework/filter#24

@csthomas
Copy link
Contributor

@tonypartridge
Copy link
Contributor

tonypartridge commented May 10, 2017

This is also causing issues with JRequest. for example:

$array = JRequest::get('request', JREQUEST_ALLOWHTML);

With a long string within the request that has UTF8 characters causes issues.

Whilst replacing it with JInput it works i.e.:

$array = $jinput->getArray(array(), null, 'HTML');

Which we should do anyhow works, the mb_string issue is still a BC.

@renekliment
Copy link

This is a really huge BC (introduced in Joomla 3.7.0) from Joomla 3.6.5 which broke our website. It broke our homepage - not sure why other people are not experiencing it - maybe some extension?

Anyway, as a temporary & ugly fix I had to replace all mb_ functions in core.php with their non-mb alternative. It's ugly, but at least our HP loads for now.

@mbabker
Copy link
Contributor

mbabker commented May 10, 2017

This is a really huge BC (introduced in Joomla 3.7.0)

A bug or regression does not automatically equate to a B/C issue.

@csthomas
Copy link
Contributor

I do not have any example about security issue in 3.6.5 version.
Can I ask question about security of below.

  1. Revert input.php file to version 3.6.5
  2. Remove invalid utf-8 bytes at the top of clean method from $source as below.
diff --git a/libraries/joomla/filter/input.php b/libraries/joomla/filter/input.php
index 99fb1d5f48..f7ac2105cb 100644
--- a/libraries/joomla/filter/input.php
+++ b/libraries/joomla/filter/input.php
@@ -144,6 +144,25 @@ class JFilterInput extends InputFilter
                        $source = $this->stripUSC($source);
                }
 
+               // Workaround for php 5.3
+               if (!defined('ENT_SUBSTITUTE'))
+               {
+                       define('ENT_SUBSTITUTE', ENT_IGNORE);
+               }
+
+               // Remove invalid UTF-8 bytes and replace it by U+FFFD
+               if (is_array($source))
+               {
+                       foreach ($source as $k => $v)
+                       {
+                               $source[$k] = htmlspecialchars_decode(htmlspecialchars($v, ENT_SUBSTITUTE, 'UTF-8'));
+                       }
+               }
+               else
+               {
+                       $source = htmlspecialchars_decode(htmlspecialchars($source, ENT_SUBSTITUTE, 'UTF-8'));
+               }
+
                // Handle the type constraint cases
                switch (strtoupper($type))

@photodude
Copy link
Contributor

@csthomas I can't speak to the "security" side. But rather than adding conditionals to Remove invalid utf-8 bytes at the top of just the clean method; shouldn't we be adding a full method to Remove invalid utf-8 bytes and call that at the top of each of the other methods. similar to how we how stripUSC at the top of the clean method?

@SniperSister
Copy link
Contributor

The security fix didn't only apply to invalid utf8-bytes but also to valid ones.

@csthomas
Copy link
Contributor

@photodude Yes, it should be move to separate method, but I want to show a simple change only, PoC.
@SniperSister Thanks, this answer I needed.

@photodude
Copy link
Contributor

@csthomas testing now on the framework from your example code
https://github.com/photodude/filter/blob/patch-3/src/InputFilter.php

@photodude
Copy link
Contributor

photodude commented May 10, 2017

@csthomas looks like that caused a whole different set of issues. https://travis-ci.org/photodude/filter/jobs/230831070
example issue from Remove invalid utf-8 bytes

--- Expected
+++ Actual
@@ @@
-'<img /> '
+'mr '

@tonypartridge
Copy link
Contributor

Some example text which is breaking the filter that maybe of help when running tests against JFilter Input Clean.
JFilterInputerCleanMaxExeciutionErrorText.txt

@lyquix-owner
Copy link

lyquix-owner commented May 10, 2017

Not sure if this is caused by the same problem, but I am experiencing a similar problem when saving an article on FLEXIcontent, as Super Admin, by just adding a simple HTML table with 15 rows.

I get a PHP timeout error: Fatal error: Maximum execution time of 60 seconds exceeded in /srv/www/example.com/public_html/libraries/vendor/joomla/string/src/phputf8/mbstring/core.php on line 96

You can see the complete POST request here: https://gist.github.com/lyquix-owner/c8c189a016c3d99c1008059920a87787

System info:
Joomla 3.7.0
PHP 7.0.5
Apache 2.4.18
Linux Ubuntu 16.04.04
DB MariaDB 5.5.5-10.0.29

@ggppdk - check this

@csthomas
Copy link
Contributor

csthomas commented May 10, 2017

IMO The bug is not directly related to UTF-8 but in UTF-8 it is more visible. The whole cleanTags() method is to check and repair.

@photodude
Copy link
Contributor

@csthomas I agree cleantags is the issue area

@ggppdk
Copy link
Contributor

ggppdk commented May 11, 2017

It is as @csthomas says,

JFilterInput::escapeAttributeValues() and JFilterInput::cleanTags() are broken because of the new multi-byte code adding byte offsets to character offsets,

The infinite loop is caused by JFilterInput::escapeAttributeValues()
-- it fails and escapes valid tags by wrong detecting them as being inside attribute values

and the above leads JFilterInput::cleanTags() to continues modify its given string, which results in the infinite loop inside JFilterInput::remove()

@ggppdk
Copy link
Contributor

ggppdk commented May 11, 2017

I have made a PR here: #15966

@photodude
Copy link
Contributor

With the PR merged can this issue be closed?

@rdeutz rdeutz closed this as completed May 14, 2017
@zero-24 zero-24 removed this from the Joomla 3.7.1 milestone May 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests