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

[Dion Hulse (dd32)]

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