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

[Edward Caissie]

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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110924/254af0de/attachment.htm>