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

Thu Jan 30 17:13:31 UTC 2014

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
> ************************************************