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
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.?
Franz 2012-06-09 15:34:52
- Milestone set to 1.5.1.
yoorick 2012-06-11 21:19:33
- Uploaded patch functions.php.diff. (view)
- Owner set to quy.
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-08-21 22:00:41
The redirect() function is used quite often in mods etc.
- Status changed from open to fixed.
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.