[theme-reviewers] Fwd: [WordPress Themes] #1596: THEME: Simply Works Core - 1.3.3

[Chip Bennett]

I think that, unless stated otherwise, that the *License* and *License
URI*header tags cover the Theme as a whole.

Chip

On Wed, Oct 20, 2010 at 9:44 AM, Edward Caissie <edward.caissie at gmail.com>wrote:

> A lot of follow-up reading to be done there, and some learning curves to be
> straightened out, too.
>
> Interesting last note about the PHP being released under the GPL, that
> should be added to the license declarations ... especially for the WP3.1
> Theme Review guidelines.
>
>
> Cais.
>
> On Wed, Oct 20, 2010 at 10:06 AM, Chip Bennett <chip at chipbennett.net>wrote:
>
>> All,
>>
>> Please see Nacin's security review comments below.
>>
>> Perhaps a good starting point for developing some security-related
>> Guidelines? (Note: this is mostly above my pay grade. I'll be in
>> watch-and-learn mode. But I definitely think we need some Theme-Review
>> guidance wrt security.)
>>
>> And, thanks to Nacin for taking the time to perform the security review!
>>
>> Chip
>>
>> ---------- Forwarded message ----------
>> From: WordPress Themes <theme-reviewers at lists.wordpress.org>
>> Date: Wed, Oct 20, 2010 at 8:53 AM
>> Subject: Re: [WordPress Themes] #1596: THEME: Simply Works Core - 1.3.3
>> To:
>>
>>
>> #1596: THEME: Simply Works Core - 1.3.3
>>
>> --------------------------+-------------------------------------------------
>>  Reporter:  simplyworks  |      Owner:  nacin
>>      Type:  theme        |     Status:  assigned
>> Resolution:               |   Keywords:  theme-simply-works-core,
>>
>> --------------------------+-------------------------------------------------
>>
>> Comment(by nacin):
>>
>>  In terms of security: Better than most, but still not great. Please read
>>  http://codex.wordpress.org/Data_Validation.
>>
>>  - User input into text options fields are not being sanitized. Seeing a
>>  lot of REQUEST on the same line as update_option. That's no good.
>>
>>  - check_admin_referer() isn't enough. You need current_user_can checks
>>  there too.
>>
>>  - Seeing a lot of form fields whose data/values/names etc isn't being
>>  escaped properly.
>>
>>  - bloginfo('name'); can't be used in the RSS feed title or any attribute,
>>  as there are a few instances of it, without getting esc_attr(). Otherwise
>>  it may escape out of the attribute.
>>
>>  - get_stylesheet_directory() exposes the path. You don't want that. You
>>  want the URL.
>>
>>  Non-security notes:
>>
>>  - Custom header should be used instead of swc_logo.
>>
>>  - in index.php, you have an is_attachment() inside of an is_single().
>>  That will never be true. single, page, and attachment are all mutually
>>  exclusive. They do however all mean is_singular() == true.
>>
>>  - comment.php doesn't need the SCRIPT_FILENAME stuff.
>>
>>  - Not a fan of the direct query in list_most_commented(), or the non-
>>  prefixed custom functions -- register_my_menus() isn't good enough IMO.
>>
>>  - In addition to the non-prefixed functions, there's for example a global
>>  called "$options" in dashboard.php. That needs to be prefixed, or
>>  preferably even not just sitting there as a global. Same with themename
>>  and shortname -- avoid globals where possible.
>>
>>  - Some strings aren't translated. Many are also not translatable, and you
>>  should instead use sprintf syntax instead of having dangling strings such
>>  as "By: ". Also, avoid escaping in translated strings, it annoys
>>  translators. Also, some strings such as "ADD COMMENT" should be "Add
>>  Comment", then text-transform:uppercase via CSS. This is better for all
>>  involved, including accessibility reasons.
>>
>>  - Don't delete options that store yes/no values. You're causing
>>  unnecessary queries as then those values are not autoloaded. Instead,
>>  store the literal value.
>>
>>  - add_theme_page() should not use `__FILE__`. Be more descriptive. Also,
>>  edit_themes should be edit_theme_options. (Or switch_themes in pre-3.0.)
>>  edit_themes controls *ONLY* the theme editor and may very well be denied
>>  to everyone in many environments, like multisite.
>>
>>  - "The CSS, XHTML and design is released under GPL" -- and the PHP?
>>
>> --
>> Ticket URL: <http://themes.trac.wordpress.org/ticket/1596#comment:3>
>> WordPress Themes <http://themes.trac.wordpress.org/>
>> WordPress.org Theme Directory Reviews
>>
>>
>> _______________________________________________
>> 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/20101020/73604443/attachment.htm>