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

[Mike Little]

On Sun, Sep 25, 2011 at 00:58, Sayontan Sinha <sayontan at gmail.com> wrote:

> 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'])"
>>
>>
>>
This line has also been in every version of WordPress since 0.7
Historically, the file wp-comments.php used to be in the root of the
WordPress directory (i.e. in the same place as




> 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.
>
>
> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
>


-- 
Mike Little
http://zed1.com/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110925/3cb00e3c/attachment-0001.htm>