Review problem - words that contain "on" and brackets after.

[Ventsi Genchev]

Virtuemart 3.4.2 ~ 3.6.2.10159

The file /administrator/components/com_virtuemart/models/ratings.php contains protection for onclick, onload and more...

Code Select Expand

$value = (string)preg_replace('#on[a-z](.+?)\)#si','',$value);//replace start of script onclick() onload()...

This protection removes all words that contain "on" and has brackets afterwards. Specifically everything after "on" to the brackets inclusive.
The problem is that this protection creates many problems.

Here are 2 examples:
User writes: "I was the only one to write a review (with rating and comment). So I have a question."
And the following is published: "I was the . So I have a question."

User writes: "A good configuration (in my opinion) is desirable."
And the following is published: "A good c is desirable."

I have now found a foolish method to avoid this and at the same time not to lose the protection of "on...". But the decision is temporary and I will wait for the team to give an opinion.

[Jörgen]

If You make this in two steps.

1. First remove white space.
2. check for "on("

If On( is not found then OK else some restoring before deleting the on( construct or rejecting this text totally because it is injecting bad code !

Jörgen @ Kreativ Fotografi

[Ventsi Genchev]

Yes, Jörgen, but this file is in the core and it is good to be permanently changed by the team.

[Ventsi Genchev]

By the way, I don't think that just "on(" will be enough.
Because it can be onclick or onload and then something in brackets. That is why such protection has been made. But maybe it should be better to avoid the problems described above.

[Jörgen]

This is of course academical right now, but search for the pattern according to how js interpretets the code a you have your answer. Your pattern search is to wide and takes in a lot of other valid constructs. I am not good at using preg and do not have the time to dig deeper into this, check js syntax and you will have your answer.

Jörgen @ Kreativ Fotografi

[Ventsi Genchev]

Thank you Jörgen for your reply.

Unfortunately, this search for a model is not mine but Max's. So I'm signaling what's not ok for the team to keep in mind.

As I wrote in the first post, I found a temporary and stupid solution that works for now. Now I'll just wait for the team to find the right solution if they want.

[GJC Web Design]

yes .. a bit of misunderstanding going on here  .. the code that Ventsi is referring to is core VM ..
But it is catching too much..

what was your temp solution V?

[Ventsi Genchev]

what was your temp solution V?

I warned the method was stupid. So don't laugh.

I replace the line:

Code Select Expand

$value = (string)preg_replace('#on[a-z](.+?)\)#si','',$value);//replace start of script onclick() onload()...

With:

Code Select Expand

if (preg_match('#on[a-z](.+?)\)#si', $value)) {
$banned = array('/on/', '/On/');
$replace = array('оn', 'Оn'); // Cyrillic 'o' and 'O'
$value = preg_replace($banned, $replace, $value);
}

In practice, under the same condition, I do not delete everything, but replace the letters "o" and "O" with the same in Cyrillic.
This way, the script cannot be executed, while at the same time the information remains.
True, not good for search engine indexing, but I prefer to have a meaningful review rather than a cropped one. Because when the review gets cut, customers are not happy. At the same time, indexing also makes no sense.

Unfortunately, this is the only solution I could think of right now.

[Studio 42]

See my last answer here http://forum.virtuemart.net/index.php?topic=143614

[Ventsi Genchev]

As i know HTML is not tolerated by default. So using core Joomla filter or PHP should remove any javascript and dont need this stupid line.
Eg.

Code Select Expand

$value = filter_var($value , FILTER_SANITIZE_STRING);
See https://www.w3schools.com/php/phptryit.asp?filename=tryphp_func_sanitize_string

Yes, Patrick, this is probably the right way.
Then maybe this line should also be dropped:

Code Select Expand

$value = preg_replace('@<[\/\!]*?[^<>]*?>@si','',$data['comment']);//remove all html tags

Interestingly, this code (FILTER_SANITIZE_STRING) is used in several places in Virtuemart. I don't know why it's not used in reviews. There may be some reason.

However, Max has to make a decision, because as I wrote earlier, this file is in the core.

[Milbo]

We changed the filters long time ago, and let the old stuff in there. So a simple vRequest::getWord should be correct then.

So just try

Code Select Expand


// no HTML TAGS but permit all alphabet
/*$value = preg_replace('@<[\/\!]*?[^<>]*?>@si','',$data['comment']);//remove all html tags
$value = (string)preg_replace('#on[a-z](.+?)\)#si','',$value);//replace start of script onclick() onload()...
$value = trim(str_replace('"', ' ', $value),"'") ;
$data['comment'] = (string)preg_replace('#^\'#si','',$value);//replace ' at start
$data['comment'] = nl2br($data['comment']);  // keep returns*/

$data['comment'] = trim(vRequest::getWord('data'));
[code]

How behave new lines? I mean, I wonder, getWord uses also FILTER_FLAG_STRIP_LOW

Okey I tested it, we need this
[code]
$data['comment'] = vRequest::filter($data['comment'],FILTER_SANITIZE_STRING, array());
$data['comment'] = ShopfunctionsF::vmSubstr($data['comment'], 0, VmConfig::get('vm_reviews_maximum_comment_length', 2000)) ;
$data['comment'] = nl2br($data['comment']);


Please test

[Ventsi Genchev]

Okey I tested it, we need this

Code Select Expand


$data['comment'] = vRequest::filter($data['comment'],FILTER_SANITIZE_STRING, array());
$data['comment'] = ShopfunctionsF::vmSubstr($data['comment'], 0, VmConfig::get('vm_reviews_maximum_comment_length', 2000)) ;
$data['comment'] = nl2br($data['comment']);


Please test

No, Max. This code does not work. Anything can be published.

If the following is written:
Welcome <script> alert(\"Hi virtuemart\")</script>
publish everything.

But if I put the top of your code:
$data['comment'] = 'Welcome <script> alert(\"Hi virtuemart\")</script>';
publish: Welcome alert(\"Hi virtuemart\")


And please change vm_reviews_maximum_comment_length with reviews_maximum_comment_length.

[Studio 42]

Why using a virtuemart function if native PHP function is easier to use ?
Try

Code Select Expand

         $data['comment'] = filter_var($data['comment'] , FILTER_SANITIZE_STRING);
         $data['comment'] = ShopfunctionsF::vmSubstr($data['comment'], 0, VmConfig::get('reviews_maximum_comment_length', 2000)) ;
         $data['comment'] = nl2br($data['comment']);

[Ventsi Genchev]

Patrick, no change. I also tried this before writing. The same effect I described above.

Please note in the post above where I described that if the text is inserted into the php file then it works.
For example, if you put this above: $data['comment'] = 'Welcome <script> alert(\"Hi virtuemart\")</script>';

Which means the FILTER_SANITIZE_STRING works, but obviously there is a problem with the text that is assigned to the $data['comment'] when writing a review.

[Ventsi Genchev]

I tested the changes to the file (ratings.php) - revision 10172, which are actually the same ones Max wrote about.

In my opinion this does not work. Does not remove any codes from the review.

Also, vm_reviews_maximum_comment_length has not yet been changed to reviews_maximum_comment_length. Until that changes, the maximum number of characters selected by the administration will not work.