[theme-reviewers] Direct access prevention in comments.php - required or recommended?

Sayontan Sinha sayontan at gmail.com
Sat Sep 24 23:58:42 UTC 2011


The Codex documentation for threaded comments actually includes this script,
hence a majority of the themes have it. See
http://codex.wordpress.org/Migrating_Plugins_and_Themes_to_2.7/Enhanced_Comment_Display.


On Sat, Sep 24, 2011 at 10:04 AM, Edward Caissie
<edward.caissie at gmail.com>wrote:

> Perhaps, interesting to note the snippet is used in about a dozen themes:
>
> -- Google ... site:http://themes.trac.wordpress.org/ "'comments.php' ==
> basename($_SERVER['SCRIPT_FILENAME'])"
>
>
> Cais.
>
>
>
> On Sat, Sep 24, 2011 at 11:06 PM, Justin Tadlock <justin at justintadlock.com
> > wrote:
>
>> **
>> Basically, the code is ultimately irrelevant.  There's no security issue
>> though.
>>
>> It's good to point it out to theme devs that they don't need it.
>>
>>
>> On 9/23/2011 9:52 PM, Dion Hulse (dd32) wrote:
>>
>> The use-case mentioned is completely different from what Mark's post is
>> about. the snippet mentioned prevents direct access to the comments
>> template, Marks post is about using $_SERVER vars in theme output.. so you
>> can ignore that posting
>>
>>  Speaking from a technical point of view (and not one of knowing what the
>> exact theme review guidelines are):
>> Having that in there is pointless, but having it in there is not a problem
>> either. It's just extra code being run, it's never going to prevent a
>> problem, nor create one (Well, unless the theme is doing strange things in
>> the comments template - like writing files or something - things that that
>> file should never do).
>> So the guidelines are suggesting removal of a useless piece of code. No
>> need to require it's removal, it's something like doing this: if ( false )
>> {run_some_code();} - it's code thats never going to be of use.. Only thing
>> removing it does it make people more aware of what the PHP is actually
>> doing..
>>
>>  As for using $pagenow/wp_die(), it's not needed, nor would it be
>> available in that case either (direct access of the file). If the comments
>> template is included, the theme is asking/WordPress is asking to output the
>> comments... bit of a no-brainer that it wouldn't need to check the current
>> templating action.
>>
>>
>> On 24 September 2011 11:37, Chip Bennett <chip at chipbennett.net> wrote:
>>
>>> I can't say that I agree that it's a security risk; it's a conditional,
>>> not an input/output.
>>>
>>>  However, I'm not sure it's really *needed*. What is the inherent risk
>>> of loading comments.php directly?
>>>
>>>  If it *is* needed, what about using $pagenow instead (I assume it's
>>> available in the front-end)? e.g.:
>>>
>>>  global $pagenow;
>>> if ( 'comments.php' = $pagenow ) {}
>>>
>>>
>>>  Also, might it be worthwhile to use wp_die() instead of die()?
>>>
>>>  Chip
>>>
>>>
>>> On Fri, Sep 23, 2011 at 8:24 PM, Tyler Cunningham <
>>> seizedpropaganda at gmail.com> wrote:
>>>
>>>>  You are correct in requiring this. It is actually now a security risk
>>>> as pointed out by Mark Jaquith in a blog post. You can link to this post if
>>>> you like:
>>>>
>>>>
>>>> http://markjaquith.wordpress.com/2009/09/21/php-server-vars-not-safe-in-forms-or-links/
>>>>
>>>> Regards,
>>>>
>>>>  Tyler Cunningham  |  Founder, COO - CyberChimps LLC<http://CyberChimps.com/>
>>>>
>>>>  @tylerbcunning <http://twitter.com/tylerbcunning>
>>>>  http://gplus.to/tylercunningham
>>>> http://linkedin.com/in/tylerbcunningham
>>>> tyler at cyberchimps.com
>>>>
>>>>   On Friday, September 23, 2011 at 6:23 PM, Vicky Arulsingam wrote:
>>>>
>>>>   I'm seeking clarification regarding the use of:
>>>>
>>>>  if ( 'comments.php' == basename($_SERVER['SCRIPT_FILENAME']) )
>>>>  die ( 'Please do not load this page directly. Thanks.' );
>>>>
>>>>  I've been requiring that themes not include this. Am I correct in
>>>> doing so or is the removal merely a recommendation?
>>>>
>>>>  -----
>>>> Vicky Arulsingam
>>>>
>>>>   _______________________________________________
>>>> 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 listtheme-reviewers at lists.wordpress.orghttp://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
>
>


-- 
Sayontan Sinha
http://mynethome.net | http://mynethome.net/blog
--
Beating Australia in Cricket is like killing a celebrity. The death gets
more coverage than the crime.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110924/95fce5d1/attachment-0001.htm>


More information about the theme-reviewers mailing list