Problem in article management (content changed after saving)

[mikesamar]

Hello,

There is a problem with Joomla 1.6.3 when saving an article that has =' in its content.
If you create a simpe article with the following 2 characters: equal simpe quote
='
When you save the article you have: equal simple quote simple quote
=''

This bug is for me a big problem. My content Google maps plugin uses for it's parameters quotes in it's tag:
For example: {mosmap lat='52.4'|lon='4.5'}

I get a lot of questions and problems that my plugin kills the layout of the
articles.

There is an issue created: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=25729

Can this bug be solved quickly!
How can I give this more attention?

See also:
http://forum.joomla.org/viewtopic.php?f=579&t=614937

Let me know if there are questions.

Kind regards,

Mike

[Mark Dexter]

You could propose a patch for it. However, the next planned release is version 1.7.0 in July. But if you get a patch and it is successfully tested, then you know it will be in the next version. Thanks. Mark

--
You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
To post to this group, send an email to joomla-...@googlegroups.com.
To unsubscribe from this group, send email to joomla-dev-cm...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/joomla-dev-cms?hl=en-GB.

[mikesamar]

I hope that somebody can find the cause and propose a patch. I'm not familiar with all the development of Joomla components. If I've a clue then I'll look into it.

I'll advice my user of the plugin not to upgrade to version 1.6.3 and stay on Joomla 1.6.1.

Regards Mike

[elin]

If you turn your filtering to raw, does it save as expected? (This works for me). 

Of course =' makes it look like part of an sql query and in an effort to prevent abuse your text filtering may try to correct that by closing it.In my case with default blacklist filtering it actually change the content of the editor to raw html once the quote was started mid article.

If I put it at the end I will just get ='</p>' because of the TinyMCE p insert. You probably are getting ='' because your editor isn't inserting a p.

What I would suggest is that you not require =  quote marks in your plugin, but instead do something a bit more standard.

Here's a snippet from the very simple image gallery how to page that shows how they us options

Samples:

{*vsig}verysimple|width=600|right=2{/vsig*} - without the asterisks
Regardless of the settings in the backend, this gallery is shown with a width of 600 pixel (width=600). The thumbnails are below the main image and get justified to the right border of the main image (right=2)

{*vsig}verysimple|width=600|right=1|cols=2{/vsig*} - without the asterisks
Width 600 pixel (width=600), thumbnails right (right=1), in two columns (cols=2)

{*vsig}verysimple|twidth=80|theight=60|space=3|quality=80{/vsig*} - without the asterisks
Thumbnails are 80 pixel of width (twidth=80), 60 pixel of height (theight=60), there is 3 pixel space between them (space=3) and they are generated with 80% of the possible quality (quality=80)

Elin

 

[Mark Dexter]

Another popular plugin syntax is the square bracket (BBS script, I think?) stuff used in forum posts (for example, [b]bold text[b/]). So maybe something like [mytagid]stuff to be tagged[/mytagid].

It just so happens that we filter that by default. It should only be filtered if it is inside an HTML element, so it is a bug and probably should get fixed in the next release. But I agree with Elin that the choice of syntax is perhaps not the best, since it mirrors what hackers do with XSS attacks. Good luck. Mark

 

--

[mikesamar]

Hello Elin,

Thanks for the response. I think you are right that filtering set to raw will cause no problem.
I can'ty test it right now, because I've not test environment available for it. I get several emails of users of my plugin where it goes wrong.

=' can be part of a SQL query, but it can also be used for parameters. If there has to be checked for SQl queries then scan for SQL commands and not vague characters combinations.

The quote is always closed so it shouldn't be corrected.

There is no real standard for Joomla and I use the standard that has the most advances and evolved from Mambo. If you use {tag}xxx{tag} it will not be filtered in several other output formats in Joomla 1.5 and maybe 1.6 (I didn't check it for 1.6) like rss.

I need the quotes for some parameters because they can have pipe | in it self as free text.

I hope I can have a look at Joomla 1.6.3 this week.

Regards Mike

[mikesamar]

Hello,

The problem is in the /libraries/joomla/filter/filterinput.php that changed with Joomla 1.6.3.

In Joomla 1.6.1 is checking the double quotes and not the single quotes. In Joomla 1.6.3 also double quotes are checked.
So that's why I didn't noticed it before.

If I add a space after the last quote in my {mosmap} string then the mixup of html tags and quotes as I mentioned is not happening. See line 585-587.

I've looked in the code and I don't see anything that is related to sql injection prevention. The code is for checking html validity and removing of unwanted html code or misformed tags.
The check on the attributes with there quotes is checked outside the recognition of the tags. First the tags should be identified and then within the tags the attributes should be checked.

I suggest to add to the filterinput the syntax for plugins so plugin are supported. So add } as closing tag too.
Attached the changes necessary to support it.

How can I suggest it to Joomla development?

Regard Mike

[elin]

Would you post that in the tracker with a category of Joomla libraries and a description of a manual testing procedure or else put it at github with an updated unit test and then send a pull request?

Thanks,

Elin

[mikesamar]

Hello Elin,

I've created a feature isue, see: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=25900

I'll think about a description how to test it and add it later.

Regards Mike

[Mark Dexter]

Hi. This is a bug which was a side effect of a security fix. I think I have an idea for a fix for it. I'll be working on it at some point later in the week I hope and will propose a patch.

The problem is that we shouldn't be filtering that type of content unless it is inside an HTML element. Mark 

[mikesamar]

Hello Mark,

You are completly right as I already suggested. I hope yo ucan solve it.

Regards Mike

[mikesamar]

I've added the test cases:

Pre condition:
* Standard Joomla install
* Default Editor TinyMCE

Testcases:
* Create simple article place =' inside => it should result in =''
* Create simple article place {tag a=''} => it should not add quote
* Create simple article place {tag a='} -> it should add a quote and result
in {tag a='}'

Do the testcases for double quote and ] too.

[mikesamar]

Hello Mark,

May I ask some questions?

In the function _cleanTags, the first executable line is:
        // First, pre-process this for illegal characters inside attribute values
        $source = $this->_escapeAttributeValues($source);

This way your are checking the complete source and not specific attributes.

I really don't understand why this is done at this stage.
The attributes are already checked below in this _cleanTags function.

Could we not remove the entire function?

Regards Mike

[Mark Dexter]

The problem is that the main function does not adequately check for some situations, which is why I added the _escapeAttributeValues function. 

This code is a bit of a mess, to be honest, and needs to be rewritten. My intention is to try using a proper parsing library, for example HTML Purifier, to actually parse and clean up the HTML and then filter the valid html. However, this will take some time. If someone else wants to take a look at this approach, that would be great.

In the meantime, the present situation is not that bad. We are being a bit over-aggressive on the filtering, but as far as I know at the present time we are adequately filtering out malicious code.

Mark

Regards Mike

--

[Mike Reumer]

Hello Mark,

What situations where left out or did you try to solve?

Yes a bit too aggressive if somebody makes a mistake with quotes in the article it can be complete chaos because tags and quotes are added. I could imaging that somebody write ='......' in a article.

Regards Mike

2011/5/16 Mark Dexter <dexter...@gmail.com>