[theme-reviewers] Why Rigorous Review of Theme Functional Files is Important

Thu Jan 30 16:38:47 UTC 2014

If I make the The option of Custom Js available(visible) only to users with unfiltered_html capability, then the themes are good to go, right? 

Date: Thu, 30 Jan 2014 11:36:35 -0500
From: chip at chipbennett.net
To: theme-reviewers at lists.wordpress.org
Subject: Re: [theme-reviewers] Why Rigorous Review of Theme Functional Files is Important

No, you can never skip sanitization.

On Thu, Jan 30, 2014 at 11:35 AM, Srikanth Koneru <tskk79 at gmail.com> wrote:

If i add 'capability' => 'unfiltered_html' then can i skip 'sanitize_callback' => 'prefix_sanitize_integer' ?

On Thu, Jan 30, 2014 at 10:04 PM, Rohit Tripathi <rohitink at live.com> wrote:

Thanks Koveshenin.
So, if I make the The option of Custom Js available only to users with unfiltered_html capability, then the themes are good to go, right?

> Date: Thu, 30 Jan 2014 20:19:39 +0400
> From: kovshenin at gmail.com
> To: theme-reviewers at lists.wordpress.org

> Subject: Re: [theme-reviewers] Why Rigorous Review of Theme Functional Files is Important
> 
> >  I have just allowed the <script> tag in the text area. Is the script tag not acceptable at all? Or should I create a New Field, derivate of Textfield, and allow <script> in that?

> 
> As Justin pointed out earlier, you should be checking whether the
> current user can publish unfiltered html, and only then show your
> custom js fields that allow script tags. Note that an some setups,

> neither admins nor super admins have the unfiltered_html capability
> for security reasons.
> 
> Also, in my opinion, Custom CSS and especially Custom JS should not be
> allowed in themes.

> 
> On Thu, Jan 30, 2014 at 8:14 PM, Chip Bennett <chip at chipbennett.net> wrote:
> > Speaking in general terms, any Theme option must be properly

> > sanitized/validated on input, and escaped on output, as appropriate.
> > Specific sanitization/validation/escaping methods depend on the specific
> > data type.
> >
> >
> > On Thu, Jan 30, 2014 at 11:12 AM, Rohit Tripathi <rohitink at live.com> wrote:

> >>
> >> I Use options framework for my theme options. I have just allowed the
> >> <script> tag in the text area. Is the script tag not acceptable at all? Or
> >> should I create a New Field, derivate of Textfield, and allow <script> in

> >> that?
> >>
> >> Regards
> >>
> >> ________________________________
> >> Date: Thu, 30 Jan 2014 11:01:22 -0500
> >>
> >> From: chip at chipbennett.net

> >> To: theme-reviewers at lists.wordpress.org
> >> Subject: Re: [theme-reviewers] Why Rigorous Review of Theme Functional
> >> Files is Important

> >>
> >> It's example code, to show that an arbitrary script can be executed. You
> >> didn't really expect me to put actually dangerous code there, did you? :)
> >>

> >>
> >> On Thu, Jan 30, 2014 at 10:58 AM, Rohit Tripathi <rohitink at live.com>
> >> wrote:
> >>
> >> I am not sure, if asking this is lame. But, why is the entering

> >> alert('text') in the header/footer codes area, being considered as an issue?
> >>
> >> Regards
> >>
> >> ________________________________
> >> Date: Thu, 30 Jan 2014 10:40:22 -0500

> >> From: chip at chipbennett.net
> >> To: theme-reviewers at lists.wordpress.org

> >> Subject: Re: [theme-reviewers] Why Rigorous Review of Theme Functional
> >> Files is Important
> >>
> >>
> >> In many cases, the issue is the lack of inherent sanitization when using

> >> the Theme Mods API with the Theme Customizer:
> >>
> >> http://make.wordpress.org/themes/2014/01/30/using-the-theme-customizer-with-the-theme-mods-api/

> >>
> >>
> >> On Thu, Jan 30, 2014 at 10:21 AM, Justin Tadlock
> >> <justin at justintadlock.com> wrote:
> >>

> >> if ( !current_user_can( 'unfiltered_html' ) ) {
> >>     /* Sanitize. */
> >> }
> >>
> >> All theme reviewers should be intimately familiar with this page:

> >> http://codex.wordpress.org/Data_Validation
> >>
> >>
> >> On 1/30/2014 7:00 AM, Chip Bennett wrote:

> >>
> >> Good morning, all,
> >>
> >> Just as a reminder why it is imperative that our reviews are thorough and
> >> complete, including a review of the Theme code and not merely a

> >> Theme-Check/front-end review, I woke up this morning to several emails
> >> reporting various Theme security vulnerabilities. Here's a sampling:
> >>
> >> To reproduce:

> >>
> >> 1. Add define( 'DISALLOW_UNFILTERED_HTML', true ); to wp-config.php
> >> 2. Activate the theme, navigate to Theme Options, add an image logo
> >> 3. In General Options - Logo Text, enter (as is, with quotes): "

> >> onclick="javascript:alert(1);"
> >> 4. Visit the homepage, click on the logo, boom.
> >>
> >> 5. In Slider Options, add a slider image and use the following for the

> >> slider text: Foo bar <script>alert('baz');</script>
> >> 6. Visit the home page, boom.
> >>
> >>
> >> To reproduce:
> >>
> >> 1. Add define( 'DISALLOW_UNFILTERED_HTML', true ); to wp-config.php

> >> 2. Activate the theme, go to Appearance - Theme Settings
> >> 3. In More Text enter: <script>alert('xss');</script>
> >> 4. Visit the home page.
> >>

> >> (you will have to have at least one post with a <!--more--> tag
> >>
> >> To reproduce:
> >>
> >> 1. Add define( 'DISALLOW_UNFILTERED_HTML', true ); to wp-config.php

> >> 2. Activate the Theme, navigate to Appearance - Theme
> >> Options - Social Netowrks Configuration
> >> 3. In Twitter URL enter: http://twitter.com/kovshenin' onclick='alert(1);'

> >> 4. Visit the home page and click the Twitter icon on the top right,
> >> ouch. Other URL fields affected too.
> >>
> >> 5. In Layout Settings - Footer enter: <script>alert(123)</script>

> >> 6. Visit the front page, ouch
> >>
> >> 7. In Advertise Settings, Header Banner Alternative: ' onclick='alert(1)'
> >> 8. Visit the front page and click the header banner, ouch

> >>
> >> 9. In Advertise Settings, Header Banner Link: http://foo.com'
> >> onclick='alert("bar")
> >> 10. Visit the front page and click the banner

> >>
> >> To reproduce:
> >>
> >> 11. In Theme Options - Integration
> >> 12. For header code: <script>alert('wow');</script>
> >> 13. Body code: <script>alert('seriously?')</script>

> >> 14. Visit the front page
> >>
> >> To reproduce:
> >>
> >> 15. in Theme Options - Colors, go to your browser JS console and
> >> enter: jQuery('#cwp_templates_topbar_colorid_color').val('blue;"

> >> onclick="javascript:alert(123);')
> >> 16. Hit save changes, visit the front page
> >> 17. The top bar is blue, try and click it. Probably all the color
> >> fields in this theme are vulnerable to this.

> >>
> >>
> >> That these issues are appearing is approved/live Themes is exactly the
> >> reason that it takes so long to get through the approved-Theme queue. We
> >> have to audit for these things, and the audits are turning into complete

> >> re-reviews in several cases.
> >>
> >> If you are uncomfortable with performing this level of review - first:
> >> don't worry. We've all been there. But the important thing is to ask for

> >> help. We have a team of 100 people, most/all of whom would be more than
> >> happy to lend a hand. We've all learned from each other. Post a comment
> >> in-ticket, or post to the mail-list, and ask for guidance. Especially when

> >> it comes to Theme options, Theme code can get quite complex and often
> >> difficult to follow. Understanding how the Settings API works sometimes
> >> seems like it requires a master's degree. And developers all have different

> >> coding styles. It's completely understandable if someone needs a second pair
> >> of eyes when reviewing a given Theme. So please: ask for help if you need it
> >> when reviewing.

> >>
> >>
> >> _______________________________________________
> >> theme-reviewers mailing list
> >> theme-reviewers at lists.wordpress.org

> >> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
> >>
> >>
> >>
> >> _______________________________________________

> >> theme-reviewers mailing list
> >> theme-reviewers at lists.wordpress.org
> >> http://lists.wordpress.org/mailman/listinfo/theme-reviewers

> >>
> >>
> >>
> >> _______________________________________________ theme-reviewers mailing
> >> list theme-reviewers at lists.wordpress.org

> >> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
> >>
> >> _______________________________________________

> >> theme-reviewers mailing list
> >> theme-reviewers at lists.wordpress.org
> >> http://lists.wordpress.org/mailman/listinfo/theme-reviewers

> >>
> >>
> >>
> >> _______________________________________________ theme-reviewers mailing
> >> list theme-reviewers at lists.wordpress.org

> >> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
> >>
> >> _______________________________________________

> >> theme-reviewers mailing list
> >> theme-reviewers at lists.wordpress.org
> >> http://lists.wordpress.org/mailman/listinfo/theme-reviewers

> >>
> >
> >
> > _______________________________________________
> > theme-reviewers mailing list
> > theme-reviewers at lists.wordpress.org

> > http://lists.wordpress.org/mailman/listinfo/theme-reviewers
> >
> 
> 
> 
> -- 
> Konstantin

> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers

_______________________________________________

theme-reviewers mailing list

theme-reviewers at lists.wordpress.org

http://lists.wordpress.org/mailman/listinfo/theme-reviewers

_______________________________________________

theme-reviewers mailing list

theme-reviewers at lists.wordpress.org

http://lists.wordpress.org/mailman/listinfo/theme-reviewers

_______________________________________________
theme-reviewers mailing list
theme-reviewers at lists.wordpress.org
http://lists.wordpress.org/mailman/listinfo/theme-reviewers 		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20140130/ea4a7d22/attachment-0001.html>