WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.54k stars 479 forks source link

XSS EscapeOutput: should we sniff for throw ? #884

Closed jrfnl closed 1 year ago

jrfnl commented 7 years ago

Just like die() or exit(), throw will send to the screen unless the Exception is caught.

Should we start checking throw statements for output escaping ?

throw new Exception( "There could be $something $evil here." );

Ref: http://php.net/manual/en/language.exceptions.php

JDGrimes commented 7 years ago

Interesting suggestion.

I'd say the difference here is that exit() is pretty much guaranteed to print to the screen, while is there is generally a good chance that an exception would be caught.

But I guess there is still the potential danger, and so escaping would be a good precaution (like we do with functions used to trigger errors).

One issue would be that an exception object could be constructed prior to actually being thrown. So you'd end up with code like this:

throw $exception;

But we could call that an edge-case and flag it as a warning or something.

jrfnl commented 7 years ago

I'd say the difference here is that exit() is pretty much guaranteed to print to the screen, while is there is generally a good chance that an exception would be caught.

What with nearly every PHP native error having been turned into an exception in PHP 7, I'd say very few are actually caught ....

But I suppose you're talking about userland Exceptions :sunglasses:

westonruter commented 7 years ago

Note that trigger_error() is also considered a printing function in WPCS, and it behaves like throwing exceptions.

UPDATE: As @JDGrimes says above:

But I guess there is still the potential danger, and so escaping would be a good precaution (like we do with functions used to trigger errors).

😄

jrfnl commented 7 years ago

My question is of course related to #764 / making the sniffs compatible with modern PHP code.

westonruter commented 7 years ago

Aside: throw is supported back to PHP 5.0.0: https://3v4l.org/uAHk0

jrfnl commented 7 years ago

@westonruter :100: I realized that as soon as I hit "Comment" (of course) ;-)

westonruter commented 7 years ago

My email has proof of this 😄

jrfnl commented 7 years ago

:blush: Note to self: ALWAYS look EVERYTHING up. Now I need :wine_glass:

jrfnl commented 7 years ago

GH should send those emails out with a five minute delay or something - I always catch a spelling mistake or something else after I post (even after proof reading)