[theme-reviewers] Why Rigorous Review of Theme Functional Files is, Important (Chip Bennett)

Thu Jan 30 17:16:23 UTC 2014

> Imagine you switch to a new theme, and your custom css (plugin) that written for previous theme mess up the new active theme.

That's not a concern :) A good Custom CSS plugin will save your CSS on
a per-theme basis, so you start fresh when you activate a new theme.

On Thu, Jan 30, 2014 at 9:13 PM, Hence Wijaya <hence.wijaya at gmail.com> wrote:
> I agree with Chip. Anyway, custom CSS is theme related feature. Imagine you
> switch to a new theme, and your custom css (plugin) that written for
> previous theme mess up the new active theme. That's not always the case but
> it might happen
>
> On 1/30/2014 11:57 PM, theme-reviewers-request at lists.wordpress.org wrote:
>>
>> Date: Thu, 30 Jan 2014 11:57:24 -0500
>> From: Chip Bennett <chip at chipbennett.net>
>> To: "Discussion list for WordPress theme reviewers."
>>         <theme-reviewers at lists.wordpress.org>
>> Subject: Re: [theme-reviewers] Why Rigorous Review of Theme Functional
>>         Files is Important
>> Message-ID:
>>
>> <CAPdLKqfri827i_5hOXJnDzTX6hN=2GY21voiVepjLb5XYUUuNg at mail.gmail.com>
>> Content-Type: text/plain; charset="iso-8859-1"
>>
>> The existence of custom CSS plugins, IMHO, is not sufficient justification
>> to force Theme developers to remove custom CSS Theme Options.
>>
>>
>> On Thu, Jan 30, 2014 at 11:54 AM, Konstantin Kovshenin
>> <kovshenin at gmail.com>wrote:
>>
>>>> I really won't recommened Adding Custom CSS to Plugins Territory.
>>>
>>> Because, In support Forums, sometimes I provide users with Options to
>>> make
>>> modifications to css which are specific to the theme. So, a Custom CSS
>>> Option is a Great Help.
>>>
>>> That's not a valid point because there are plenty of great and secure
>>> Custom CSS plugins out there.
>>>
>>> On Thu, Jan 30, 2014 at 8:50 PM, Rohit Tripathi <rohitink at live.com>
>>> wrote:
>>>>
>>>> I really won't recommened Adding Custom CSS to Plugins Territory.
>>>
>>> Because,
>>>>
>>>> In support Forums, sometimes I provide users with Options to make
>>>> modifications to css which are specific to the theme. So, a Custom CSS
>>>> Option is a Great Help.
>>>>
>>>> Random Header/Footer Scripts Could be considered Plugin Territory.
>>>>
>>>>> Date: Thu, 30 Jan 2014 20:46:06 +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
>>>>>
>>>>>> Sneak peek: when we begin discussing Guidelines Revisions (next week),
>>>>>> I'm going to propose that options for arbitrary header/footer scripts
>>>
>>> are
>>>>>>
>>>>>> Plugin Territory.
>>>>>
>>>>> Thank you! Can we add CSS to that? :)
>>>>>
>>>>> On Thu, Jan 30, 2014 at 8:44 PM, Chip Bennett <chip at chipbennett.net>
>>>>> wrote:
>>>>>>
>>>>>> Sneak peek: when we begin discussing Guidelines Revisions (next week),
>>>>>> I'm
>>>>>> going to propose that options for arbitrary header/footer scripts are
>>>>>> Plugin
>>>>>> Territory.
>>>>>>
>>>>>>
>>>>>> On Thu, Jan 30, 2014 at 11:40 AM, Konstantin Kovshenin
>>>>>> <kovshenin at gmail.com>
>>>>>> wrote:
>>>>>>>>
>>>>>>>> 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?
>>>>>>>
>>>>>>> That would make the theme more secure, yes, but ultimately it's up to
>>>>>>> the reviewers and the guidelines on whether it's good to go or not.
>>>>>>>
>>>>>>> Again, my opinion is that Custom CSS and Custom JS are plugin
>>>>>>> territory, which would stop theme authors from reinventing the wheel.
>>>>>>> It would also mean that reviewers will spend less time auditing theme
>>>>>>> code. And it would also mean that there's less risk of insecure code
>>>>>>> ending up in the themes directory. Right, let's dump it all in the
>>>>>>> plugins directory :)
>>>>>>>
>>>>>>> On Thu, Jan 30, 2014 at 8:34 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
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> 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
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>>
>>>
>>>
>>> --
>>> Konstantin
>>> _______________________________________________
>>> 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/b9b9fc30/attachment.html>
>>
>> ------------------------------
>>
>> Subject: Digest Footer
>>
>> _______________________________________________
>> theme-reviewers mailing list
>> theme-reviewers at lists.wordpress.org
>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>
>>
>> ------------------------------
>>
>> End of theme-reviewers Digest, Vol 44, Issue 145
>> ************************************************
>
>
> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers

-- 
Konstantin