Ticket #681 (fixed bug)
Redirect and execution termination
- Created: 2012-06-08 20:17:44
- Reported by: yoorick
- Assigned to: quy
- Milestone: 1.5.1
- Component: code
- Priority: normal
In login.php, lines 231 and 115, there seems exit; statement to be missing after header('Location ...'); calls. It leads to rendering of "invisible" pages.
Also, I believe that in functions.php in redirect() function code on line 1300 should use exit; statement as well. If I am wrong and it is important to have rendered redirect page along with Location header sent (e.g. browser not supporting redirection), then maybe you should consider using redirect() function instead of header('Location ...'); in places like login.php or viewtopic.php or etc.?
History
Based on these 2 calls and other calls to the redirect function, as seen the URL parameter is escaped in the call and not within the function.
redirect(htmlspecialchars($_POST['redirect_url']), $lang_login['Login redirect']);
redirect(htmlspecialchars($_POST['redirect_url']), $lang_misc['Email sent redirect']);Therefore, this can be skipped:
<?php echo str_replace(array('<', '>', '"'), array('<', '>', '"'), $destination_url) ?>Please confirm to remove the above code.
Franz 2012-08-06 09:25:33
I agree, though maybe we should just move the escaping to the function instead?
yoorick 2012-08-21 20:51:09
Escaping inside the redirect() function would be more convenient for new code because you do not have to explicitly escape it outside. But what about any existing code that is not in FluxBB's core (mods, plugins, etc.), that uses the function? It's like to change an API, I think.
(Actually, I don't know if it is used very often in plugins and mods or not. So it is just my thought on the topic.)
Franz 2012-10-17 16:29:32
As a general thing: it is never good to use htmlspecialchars() without any additional parameters. That allows bad things to happen, for example un-escaped single quotes. 