I am guessing but moving up esc_url only works because the vulnerable url fails the list of allowed protocols. This will break relative urls being passed (try using /forums/ for example without http)
But since relative is a rare case (I use relative but via plugin) I guess it’s acceptable.
I am guessing but moving up esc_url only works because the vulnerable url fails the list of allowed protocols. This will break relative urls being passed (try using /forums/ for example without http)
But since relative is a rare case (I use relative but via plugin) I guess it’s acceptable.
In this topic is fine. Would rather have more eyes on this than less.
The solution involved the shuffling of things around, as well as this specific addition: https://trac.bbpress.org/browser/trunk/bb-login.php#L51
Basically if the esc’ed $re is now empty, fall back to the installation root. At first this seemed like a silly solution, but because the login always attempts to smart redirect, there isn’t a circumstance where it would naturally be empty. By moving the esc’s up and letting them filter out the baddies, it’s possible to end up with an empty $re.
Tested with ampersands and question marks and it appears to work fine. I’ll try more esoteric URL combinations and see if it breaks.
I indented that code because of the repeated empty( $re ) checks. I had a hard time keeping track of how many times it needed to repeat the same check and bumped them in for clarity. Not a common formatting technique but helpful to me at the time.
In this topic is fine. Would rather have more eyes on this than less.
The solution involved the shuffling of things around, as well as this specific addition: https://trac.bbpress.org/browser/trunk/bb-login.php#L51
Basically if the esc’ed $re is now empty, fall back to the installation root. At first this seemed like a silly solution, but because the login always attempts to smart redirect, there isn’t a circumstance where it would naturally be empty. By moving the esc’s up and letting them filter out the baddies, it’s possible to end up with an empty $re.
Tested with ampersands and question marks and it appears to work fine. I’ll try more esoteric URL combinations and see if it breaks.
I indented that code because of the repeated empty( $re ) checks. I had a hard time keeping track of how many times it needed to repeat the same check and bumped them in for clarity. Not a common formatting technique but helpful to me at the time.
Maybe I am getting rusty, but looking at the diff, I don’t see it.
Basically it’s the same thing, shuffled around a bit and the same esc_url and esc_attr is being used at the end, which is the same pattern as the existing 1.0/1.1 code.
Are you sure the fix was not being caused by just having a more updated version of esc_url and esc_attr ?
Or was it just being solved by moving those two sanitizers up higher before bb_safe_redirect could ever be called. Because that was definitely an oversight on someone’s part (not me). Based on the comments I’d guess they thought their functionality was for display filtering and not actually sanitization.
Looking at esc_url though, its default is indeed meant for displaying urls by default, note the comment in the code “Replace ampersands and single quotes only when displaying.” and how the context is set by default to “display”.
Maybe test your solution with a url that contains an ampersand, it probably will not work?
Also, are you relying on the list of protocols to do the sanitization? Because that could be defeated too (and would prevent using relative urls that are legitimate).
ps. very minor but you have confusing indentation indicating nesting with the code at line 18 – which actually is not nested
Feel free to email me directly if that’s better.
Maybe I am getting rusty, but looking at the diff, I don’t see it.
Basically it’s the same thing, shuffled around a bit and the same esc_url and esc_attr is being used at the end, which is the same pattern as the existing 1.0/1.1 code.
Are you sure the fix was not being caused by just having a more updated version of esc_url and esc_attr ?
Or was it just being solved by moving those two sanitizers up higher before bb_safe_redirect could ever be called. Because that was definitely an oversight on someone’s part (not me). Based on the comments I’d guess they thought their functionality was for display filtering and not actually sanitization.
Looking at esc_url though, its default is indeed meant for displaying urls by default, note the comment in the code “Replace ampersands and single quotes only when displaying.” and how the context is set by default to “display”.
Maybe test your solution with a url that contains an ampersand, it probably will not work?
Also, are you relying on the list of protocols to do the sanitization? Because that could be defeated too (and would prevent using relative urls that are legitimate).
ps. very minor but you have confusing indentation indicating nesting with the code at line 18 – which actually is not nested
Feel free to email me directly if that’s better.
Happy holidays to you too. 
I am confident this potential exploit is now fixed, so if anyone wants to put some eyes on the bb-login.php from trunk would be helpful.
Happy holidays to you too.
I am confident this potential exploit is now fixed, so if anyone wants to put some eyes on the bb-login.php from trunk would be helpful.
Sorry. But there is no such option available.
edit: Please don’t link offsite unless it’s bbPress related – jjj
Sorry. But there is no such option available.
edit: Please don’t link offsite unless it’s bbPress related – jjj
what code? I get the same error.
ok, i managed to install bbpress using the quickinstall item from the cpanel.
it took just 5 sec.
the bb-config file is set just like i did manually before, so i still wonder why it didnt work previously…
ok, i managed to install bbpress using the quickinstall item from the cpanel.
it took just 5 sec.
the bb-config file is set just like i did manually before, so i still wonder why it didnt work previously…
i am with hostgator too
thanks! that’s what i did. i dunno why it cant work.
pff…i thought that was easy to install….
thinking of going back to the good old PHP BB…
i am with hostgator too
thanks! that’s what i did. i dunno why it cant work.
pff…i thought that was easy to install….
thinking of going back to the good old PHP BB…
I’ll call it a win anyway 
but, really? Administrator can’t manage themes and plugins?
Zaerl that aint a one-sentence description.
Anonymous UserInactive
Oh – fault. 
thx for correction
Anonymous UserInactive
Oh – fault. thx for correction
<?php if( is_bb_admin() ) { ?> class="highlight"<?php } ?>
This is wrong Markus. is_bb_admin (which is deprecated, use bb_is_admin) returns true if the page is an administrative page. It’s just a wrapper that returns the value of the BB_IS_ADMIN constant.
<?php if( is_bb_admin() ) { ?> class="highlight"<?php } ?>
This is wrong Markus. is_bb_admin (which is deprecated, use bb_is_admin) returns true if the page is an administrative page. It’s just a wrapper that returns the value of the BB_IS_ADMIN constant.
Markus, thanks again and sorry that I didn’t find this solution over bbpress forum before my last post.
But I think it would be great and easy if it can be achieved with Zaerl’s code??
I mean we can get this solution without touching front-page.php, topic.php, post.php or other files.
And it would be also great to highlight multiple authors by their ids which zearl did for topics/forums.
want something like – $za_highlight_auhors = array(1,8,55,89); ??
Possible?
Markus, thanks again and sorry that I didn’t find this solution over bbpress forum before my last post.
But I think it would be great and easy if it can be achieved with Zaerl’s code??
I mean we can get this solution without touching front-page.php, topic.php, post.php or other files.
And it would be also great to highlight multiple authors by their ids which zearl did for topics/forums.
want something like – $za_highlight_auhors = array(1,8,55,89); ??
Possible?
Anonymous UserInactive
For individual classes for author post you can use
class="highlight_<?php post_author_id();?>"
Anonther good starting point to search:
http://phpxref.ftwr.co.uk/bbpress/nav.html?_functions/index.html
There are always different ways to achieve an objective. It is only a question of whether the target should be achieved easily, quickly or safely.
Anonymous UserInactive
For individual classes for author post you can use
class="highlight_<?php post_author_id();?>"
Anonther good starting point to search:
http://phpxref.ftwr.co.uk/bbpress/nav.html?_functions/index.html
There are always different ways to achieve an objective. It is only a question of whether the target should be achieved easily, quickly or safely.
