Ravens PHP Scripts: Forums
 

 

View next topic
View previous topic
Post new topic   Reply to topic    Ravens PHP Scripts And Web Hosting Forum Index -> RavenNuke(tm) v2.5x
Author Message
neralex
Site Admin



Joined: Aug 22, 2007
Posts: 1549

PostPosted: Tue Jul 03, 2012 9:45 pm Reply with quote

hey!

I have tested some html tags with the CK editor 3-6-3 and got a crazy issue with function check_html.

rnconfig AllowableHTML:
Code:
   'object' => array('style' => 1, 'id' => 1, 'data' => 1, 'width' => 1, 'height' => 1, 'id' => 1, 'type' => 1),   

   'param' => array('name' => 1, 'value' => 1),


my html code:

Code:
<object data="http://www.youtube.com/v/6Ws2WQKH9SE?rel=0&amp;loop=1" style="width: 500px; height: 312px;" type="application/x-shockwave-flash"><param name="wmode" value="Opaque" /><param name="movie" value="http://www.youtube.com/v/6Ws2WQKH9SE?rel=0&amp;loop=1" /></object>


result after save:

Code:
<object data="http://www.youtube.com/v/6Ws2WQKH9SE?rel=0&amp;loop=1" style="500px; height: 312px;" type="application/x-shockwave-flash"><param name="wmode" value="Opaque" /><param name="movie" value="http://www.youtube.com/v/6Ws2WQKH9SE?rel=0&amp;loop=1" /></object>


same thing with a div:

Code:
<div align="center" style="width: 500px; height: 312px;">test</div>


result after save:

Code:
<div align="center" style="500px; height: 312px;">test</div>


for saveing into the database i used this filtering:

Code:
$bodytext = $db->sql_escape_string(check_html($bodytext, ''));


If i remove check_html, then its all fine.

Question


Last edited by neralex on Wed Jul 18, 2012 3:30 am; edited 2 times in total 
View user's profile Send private message
fkelly
Former Moderator in Good Standing



Joined: Aug 30, 2005
Posts: 3312
Location: near Albany NY

PostPosted: Fri Jul 06, 2012 9:44 am Reply with quote

I can't address all the issues here but I know that as part of check_html the kses program encodes html entities. I think the term it uses is "normalizes" which has always been ambiguous and misleading to me.

So, if you have an & it will get encoded as &amp; and single and double quotes will get encoded too. Which is not good since that's not what you want stored in the database. Why and where "width" is being removed from your other code is beyond me.

You could remove ckeditor from the mix and just try a test where you set a variable to:

Code:
<div align="center" style="width: 500px; height: 312px;">test</div>


then pass it through check_html and see what results. If the problem occurs then we know it is in check_html. Though I'm not sure what we'd do since kses is unsupported software at this time. Maybe omit the checks wherever possible.
 
View user's profile Send private message Visit poster's website
neralex







PostPosted: Sat Jul 07, 2012 3:53 am Reply with quote

I've got the same result with a disabled ckeditor and i have tested with a fresh install of RN25 with the same result. Database is utf8_gerneral_ci - not converted.
 
spasticdonkey
RavenNuke(tm) Development Team



Joined: Dec 02, 2006
Posts: 1693
Location: Texas, USA

PostPosted: Sat Jul 07, 2012 9:35 am Reply with quote

I see what you are saying... I guess I would suggest using a class to set the width/height in the meantime...

It seems to be not necessarily related to the width attribute, but to the first style declared

<div style="color:black;width:300px;height:400px">abc</div>
becomes
<div style="black;width:300px;height:400px">abc</div>

I have a feeling the issue is occurring in function kses_bad_protocol and/or kses_bad_protocol_once - but that's just a guess at this point.
 
View user's profile Send private message Visit poster's website
neralex







PostPosted: Sat Jul 07, 2012 10:13 am Reply with quote

crazy - thanks for testing, too!
 
nuken
RavenNuke(tm) Development Team



Joined: Mar 11, 2007
Posts: 2024
Location: North Carolina

PostPosted: Sat Jul 07, 2012 10:26 am Reply with quote

I just tested it with the latest RN development version using HTMLPurifier and CKeditor. This doesn't seem to be an issue with it. You may want to test it too and make sure....

_________________
Tricked Out News 
View user's profile Send private message Send e-mail Visit poster's website
neralex







PostPosted: Sat Jul 07, 2012 11:48 am Reply with quote

In version RN24 there was no such problem. Is there no way to fix this issue in RN25?
 
montego
Site Admin



Joined: Aug 29, 2004
Posts: 9457
Location: Arizona

PostPosted: Sun Jul 08, 2012 9:09 am Reply with quote

I do see that there was quite a bit of change made in the check_html() function between these two versions. I am not sure why the changes were made as I was on sabbatical for that release. You might want to try to replace the check_html() function code in 2.5 with what was in 2.4 and see if it works better for you.

_________________
Where Do YOU Stand?
HTML Newsletter::ShortLinks::Mailer::Downloads and more... 
View user's profile Send private message Visit poster's website
neralex







PostPosted: Sun Jul 08, 2012 1:09 pm Reply with quote

Ok i have checked the news admin file in RN 2.40.01 and there was not used the function check_html to save the bodytext. It was used FixQuotes. But this is outdated. The issue exist only in RN25.

spasticdonkey wrote:

It seems to be not necessarily related to the width attribute, but to the first style declared


i have tested a wrong typing and here is it the same result.

entered:

Code:
<div align="width:20px">test</div>


stored:

Code:
<div align="20px">test</div>


The kses.php is exactly the same file and the old check_html function is not the solution.
 
neralex







PostPosted: Sun Jul 08, 2012 2:29 pm Reply with quote

I have found an modded kses.php. Near line 314 is an comment with extra lines. I have it tested and it works on my local RN25 but it would be great if someone check if it is clean.
[ Only registered users can see links on this board! Get registered or login! ]

Code:
          // MDL-2684 - kses stripping CSS styles that it thinks look like protocols

          if ($attrname == 'style') {
              $thisval = $match[1];
          } else {
              $thisval = kses_bad_protocol($match[1], $allowed_protocols);
          }
 
montego







PostPosted: Tue Jul 10, 2012 7:23 am Reply with quote

If I am reading that code right, it sure looks like it will now allow ANY style directives which I think is a security hole.

neralex, you say that "the old check_html function is not the solution". Have you tried swapping out that function's code from 2.4 into 2.5 and made NO OTHER CHANGES? If so, are you certain that it really did work in 2.4 previously?

You are right, FixQuotes is outdated (worthless).
 
neralex







PostPosted: Tue Jul 10, 2012 8:11 am Reply with quote

montego wrote:
Have you tried swapping out that function's code from 2.4 into 2.5 and made NO OTHER CHANGES?


Yes, i have made no changes on the function. It was an copy&paste. Its the mainfile of Palbin's ckeditor. But i think the issue comes not from the function. Try it self, please. I have tried with fresh install, too. The issue makes me sad, because i have many styles settings in my articles and this bug breaks all after a saving in RN25. It would be nice if the matter could take someone.
 
montego







PostPosted: Sat Jul 14, 2012 10:09 am Reply with quote

[quote="neralex"]
montego wrote:
Its the mainfile of Palbin's ckeditor.


Well, then that is NOT the code from 2.4's mainfile.php check_html() function as I had requested you to try.

This may have to take Palbin looking at it given you are working with his CKeditor version.
 
Palbin
Site Admin



Joined: Mar 30, 2006
Posts: 2583
Location: Pittsburgh, Pennsylvania

PostPosted: Sun Jul 15, 2012 10:57 am Reply with quote

Looking at this today.

_________________
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan. 
View user's profile Send private message
Palbin







PostPosted: Sun Jul 15, 2012 11:53 am Reply with quote

It did work in RN 2.4, but not really. In RN 2.4 we were not using check_html() in news admin. Looking for/at the root cause now.
 
neralex







PostPosted: Sun Jul 15, 2012 3:00 pm Reply with quote

Palbin wrote:
It did work in RN 2.4, but not really. In RN 2.4 we were not using check_html() in news admin. Looking for/at the root cause now.


Yes it is right! I think we need for RN25 an fix for kses.
 
Palbin







PostPosted: Sun Jul 15, 2012 4:40 pm Reply with quote

Ok I think I have this fixed.

Use this kses.php [ Only registered users can see links on this board! Get registered or login! ]

This is not required, but you should also edit line 1006 of mainfile.php(line number will vary between versions)
Code:


function check_html ($string, $allowed_html = '', $allowed_protocols = array()) {


The following protocols and style attributes should now be allowed. If you enabled styles for a particular tag all the attributes will be available for that tag. This is not configurable on a per tag basis.

Code:


$allowed_protocols = array('http', 'https', 'ftp', 'mailto', 'news', 'irc', 'gopher', 'nntp', 'feed', 'telnet', 'mms', 'rtsp', 'svn');


Code:


   $allowed_attr = array( 'text-align', 'margin', 'color', 'float',
   'border', 'background', 'background-color', 'border-bottom', 'border-bottom-color',
   'border-bottom-style', 'border-bottom-width', 'border-collapse', 'border-color', 'border-left',
   'border-left-color', 'border-left-style', 'border-left-width', 'border-right', 'border-right-color',
   'border-right-style', 'border-right-width', 'border-spacing', 'border-style', 'border-top',
   'border-top-color', 'border-top-style', 'border-top-width', 'border-width', 'caption-side',
   'clear', 'cursor', 'direction', 'font', 'font-family', 'font-size', 'font-style',
   'font-variant', 'font-weight', 'height', 'letter-spacing', 'line-height', 'margin-bottom',
   'margin-left', 'margin-right', 'margin-top', 'overflow', 'padding', 'padding-bottom',
   'padding-left', 'padding-right', 'padding-top', 'text-decoration', 'text-indent', 'vertical-align',
   'width');


Last edited by Palbin on Sun Jul 15, 2012 5:02 pm; edited 1 time in total 
hicuxunicorniobestbuildpc
The Mouse Is Extension Of Arm



Joined: Aug 13, 2009
Posts: 1015
Location: Netherland

PostPosted: Sun Jul 15, 2012 4:50 pm Reply with quote

Did you mean line 58?

Code:
function kses($string, $allowed_html, $allowed_protocols = array()) {


Replace it with this one

Code:
function check_html ($string, $allowed_html = '', $allowed_protocols = array()) {


Thanks Palbin with taking care of this issue.
 
View user's profile Send private message Visit poster's website
Palbin







PostPosted: Sun Jul 15, 2012 5:02 pm Reply with quote

That was supposed to say line 1006 of mainfile.php. I have edited my previous post.
 
hicuxunicorniobestbuildpc







PostPosted: Mon Jul 16, 2012 10:31 am Reply with quote

in RavenNuke 2.5 I have this

Code:
function check_html ($string, $allowed_html = '', $allowed_protocols = array('http', 'https', 'ftp', 'news', 'nntp', 'gopher', 'mailto')) {



Where I should put these one


Quote:
$allowed_attr = array( 'text-align', 'margin', 'color', 'float',
'border', 'background', 'background-color', 'border-bottom', 'border-bottom-color',
'border-bottom-style', 'border-bottom-width', 'border-collapse', 'border-color', 'border-left',
'border-left-color', 'border-left-style', 'border-left-width', 'border-right', 'border-right-color',
'border-right-style', 'border-right-width', 'border-spacing', 'border-style', 'border-top',
'border-top-color', 'border-top-style', 'border-top-width', 'border-width', 'caption-side',
'clear', 'cursor', 'direction', 'font', 'font-family', 'font-size', 'font-style',
'font-variant', 'font-weight', 'height', 'letter-spacing', 'line-height', 'margin-bottom',
'margin-left', 'margin-right', 'margin-top', 'overflow', 'padding', 'padding-bottom',
'padding-left', 'padding-right', 'padding-top', 'text-decoration', 'text-indent', 'vertical-align',
'width');
 
Palbin







PostPosted: Mon Jul 16, 2012 2:58 pm Reply with quote

Replace:
Code:


function check_html ($string, $allowed_html = '', $allowed_protocols = array('http', 'https', 'ftp', 'news', 'nntp', 'gopher', 'mailto')) {

with:
Code:


function check_html ($string, $allowed_html = '', $allowed_protocols = array()) {
 
hicuxunicorniobestbuildpc







PostPosted: Mon Jul 16, 2012 5:45 pm Reply with quote

Yes, I did it. What is the next step. Thanks for taking care of this important issue
 
neralex







PostPosted: Tue Jul 17, 2012 6:30 am Reply with quote

Palbin wrote:
Ok I think I have this fixed.


Palbin, let me say thank you. It works fine!

RavensScripts
 
hicuxunicorniobestbuildpc







PostPosted: Tue Jul 17, 2012 8:01 am Reply with quote

Wave it works perfect! Thanks a lot! Palbin Wink
 
neralex







PostPosted: Wed Jul 18, 2012 3:16 am Reply with quote

Palbin, I think i have found an bug in your fixed version of the kses.php:

Fatal error: Call to undefined function wp_kses_decode_entities() in F:\wamp\www\raven25\includes\kses\kses.php on line 528

The function wp_kses_no_null is undefined, too. Do you mean kses_decode_entities and kses_no_null ?


Last edited by neralex on Thu Jul 26, 2012 1:49 pm; edited 1 time in total 
Display posts from previous:       
Post new topic   Reply to topic    Ravens PHP Scripts And Web Hosting Forum Index -> RavenNuke(tm) v2.5x

View next topic
View previous topic
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You can attach files in this forum
You can download files in this forum


Powered by phpBB © 2001-2007 phpBB Group
All times are GMT - 6 Hours
 
Forums ©