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

Thu Jan 30 16:36:35 UTC 2014

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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20140130/26334950/attachment-0001.html>