A lot of follow-up reading to be done there, and some learning curves to be straightened out, too.<br><br>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.<br>

<br><br>Cais.<br><br><div class="gmail_quote">On Wed, Oct 20, 2010 at 10:06 AM, Chip Bennett <span dir="ltr">&lt;<a href="mailto:chip@chipbennett.net">chip@chipbennett.net</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

All,<div><br></div><div>Please see Nacin&#39;s security review comments below.</div><div><br></div><div>Perhaps a good starting point for developing some security-related Guidelines? (Note: this is mostly above my pay grade. I&#39;ll be in watch-and-learn mode. But I definitely think we need some Theme-Review guidance wrt security.)</div>


<div><br></div><div>And, thanks to Nacin for taking the time to perform the security review!</div><div><br></div><div>Chip<br><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">WordPress Themes</b> <span dir="ltr">&lt;<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a>&gt;</span><br>


Date: Wed, Oct 20, 2010 at 8:53 AM<br>Subject: Re: [WordPress Themes] #1596: THEME: Simply Works Core - 1.3.3<br>To: <br><br><br><div>#1596: THEME: Simply Works Core - 1.3.3<br>
--------------------------+-------------------------------------------------<br>
  Reporter:  simplyworks  |      Owner:  nacin<br>
      Type:  theme        |     Status:  assigned<br>
Resolution:               |   Keywords:  theme-simply-works-core,<br>
--------------------------+-------------------------------------------------<br>
<br>
</div>Comment(by nacin):<br>
<br>
 In terms of security: Better than most, but still not great. Please read<br>
 <a href="http://codex.wordpress.org/Data_Validation" target="_blank">http://codex.wordpress.org/Data_Validation</a>.<br>
<br>
  - User input into text options fields are not being sanitized. Seeing a<br>
 lot of REQUEST on the same line as update_option. That&#39;s no good.<br>
<br>
  - check_admin_referer() isn&#39;t enough. You need current_user_can checks<br>
 there too.<br>
<br>
  - Seeing a lot of form fields whose data/values/names etc isn&#39;t being<br>
 escaped properly.<br>
<br>
  - bloginfo(&#39;name&#39;); can&#39;t be used in the RSS feed title or any attribute,<br>
 as there are a few instances of it, without getting esc_attr(). Otherwise<br>
 it may escape out of the attribute.<br>
<br>
  - get_stylesheet_directory() exposes the path. You don&#39;t want that. You<br>
 want the URL.<br>
<br>
 Non-security notes:<br>
<br>
  - Custom header should be used instead of swc_logo.<br>
<br>
  - in index.php, you have an is_attachment() inside of an is_single().<br>
 That will never be true. single, page, and attachment are all mutually<br>
 exclusive. They do however all mean is_singular() == true.<br>
<br>
  - comment.php doesn&#39;t need the SCRIPT_FILENAME stuff.<br>
<br>
  - Not a fan of the direct query in list_most_commented(), or the non-<br>
 prefixed custom functions -- register_my_menus() isn&#39;t good enough IMO.<br>
<br>
  - In addition to the non-prefixed functions, there&#39;s for example a global<br>
 called &quot;$options&quot; in dashboard.php. That needs to be prefixed, or<br>
 preferably even not just sitting there as a global. Same with themename<br>
 and shortname -- avoid globals where possible.<br>
<br>
  - Some strings aren&#39;t translated. Many are also not translatable, and you<br>
 should instead use sprintf syntax instead of having dangling strings such<br>
 as &quot;By: &quot;. Also, avoid escaping in translated strings, it annoys<br>
 translators. Also, some strings such as &quot;ADD COMMENT&quot; should be &quot;Add<br>
 Comment&quot;, then text-transform:uppercase via CSS. This is better for all<br>
 involved, including accessibility reasons.<br>
<br>
  - Don&#39;t delete options that store yes/no values. You&#39;re causing<br>
 unnecessary queries as then those values are not autoloaded. Instead,<br>
 store the literal value.<br>
<br>
  - add_theme_page() should not use `__FILE__`. Be more descriptive. Also,<br>
 edit_themes should be edit_theme_options. (Or switch_themes in pre-3.0.)<br>
 edit_themes controls *ONLY* the theme editor and may very well be denied<br>
 to everyone in many environments, like multisite.<br>
<br>
  - &quot;The CSS, XHTML and design is released under GPL&quot; -- and the PHP?<br>
<font color="#888888"><br>
--<br>
Ticket URL: &lt;<a href="http://themes.trac.wordpress.org/ticket/1596#comment:3" target="_blank">http://themes.trac.wordpress.org/ticket/1596#comment:3</a>&gt;<br>
</font><div><div></div><div>WordPress Themes &lt;<a href="http://themes.trac.wordpress.org/" target="_blank">http://themes.trac.wordpress.org/</a>&gt;<br>
WordPress.org Theme Directory Reviews<br>
</div></div></div><br></div>
<br>_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br></blockquote></div><br>