Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XSS EscapeOutput: should we sniff for throw ? #884

Closed
jrfnl opened this issue Mar 21, 2017 · 9 comments · Fixed by #2326
Closed

XSS EscapeOutput: should we sniff for throw ? #884

jrfnl opened this issue Mar 21, 2017 · 9 comments · Fixed by #2326

Comments

@jrfnl
Copy link
Member

jrfnl commented Mar 21, 2017

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
Copy link
Contributor

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
Copy link
Member Author

jrfnl commented Mar 21, 2017

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 😎

@westonruter
Copy link
Member

westonruter commented Mar 21, 2017

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
Copy link
Member Author

jrfnl commented Mar 21, 2017

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

@westonruter
Copy link
Member

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

@jrfnl
Copy link
Member Author

jrfnl commented Mar 21, 2017

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

@westonruter
Copy link
Member

My email has proof of this 😄

@jrfnl
Copy link
Member Author

jrfnl commented Mar 21, 2017

😊 Note to self: ALWAYS look EVERYTHING up. Now I need 🍷

@jrfnl
Copy link
Member Author

jrfnl commented Mar 21, 2017

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants