Bug #1729

CSRF vulnerability in ?language=

Added by Nathan Malcolm over 2 years ago. Updated over 2 years ago.

Status:ClosedStart date:09/20/2011
Priority:NormalDue date:
Assignee:Diogo Parrinha% Done:

100%

Category:-
Target version:1.6.5
Reproducibility:Always Database Type:
Reported In MyBB Version:1.6.4 Database Version:
PHP Version:5.3.8 SQA assignments:Jitendra Maharaj
Browser:

Description

PoC - http://community.mybb.com/thread-104279.html

Using http://community.mybb.com/forum-13.html?language=pirate as an image will change the users language to the specified value.

There was a similar vulnerability with polls and marking forums as read, which is the same concept. If a user was to post that on a large forum, many users languages could be changed without their permission. If the user doesn't speak that language, chances are they won't know how to change it back.

History

#1 Updated by Diogo Parrinha over 2 years ago

  • Status changed from New to Assigned
  • Assignee set to Diogo Parrinha
  • Target version set to 1.6.5

I consider this CSRF vulnerability only because it is possible to change user data.
Where does it need updating? Dropdown box and what else? I can't think of any other place which allows us to switch to a different language pack.

#2 Updated by Nathan Malcolm over 2 years ago

There are two known places - The user cp, which I assume already has the post key in place, and the drop down in the bottom right hand corner where it allows the language to be changed from any page. I believe this is the culprit.

Lines 64 - 92 in global.php is where it needs to be patched to check for the post key, and this would also require a template edit for a hidden field in footer_languageselect.

#3 Updated by Diogo Parrinha over 2 years ago

Going to fix this issue in a minute.

#4 Updated by Diogo Parrinha over 2 years ago

By the way my solution is to check the post key in passive mode and only change the language if the key is valid. Otherwise do nothing. It must be done this way because the templates are not loaded at that time thus we can't show any errors.

#5 Updated by Diogo Parrinha over 2 years ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

Applied in changeset r5632.

#6 Updated by Tom Moore over 2 years ago

You don't need the && verify_post_check($mybb->input['postcode'], true) in the if statement. It could be a good idea to rename "postcode" in the theme XML to "my_post_key" to keep it inline with other postcode checks too.

#7 Updated by Diogo Parrinha over 2 years ago

Actually I do. The one inside the if was a mistake and would not work properly. Preparing fix.

#8 Updated by Diogo Parrinha over 2 years ago

  • Status changed from Resolved to Assigned

#9 Updated by Diogo Parrinha over 2 years ago

  • Status changed from Assigned to Resolved

Applied in changeset r5633.

#10 Updated by Diogo Parrinha over 2 years ago

  • Status changed from Resolved to Assigned

Damn forgot my_post_key in the code.

#11 Updated by Diogo Parrinha over 2 years ago

Applied in changeset r5634.

#12 Updated by Diogo Parrinha over 2 years ago

  • Status changed from Assigned to Resolved

#13 Updated by Tom Moore over 2 years ago

The other way around lol. This:

if($mybb->input['language'] && $lang->language_exists($mybb->input['language']))
{
    verify_post_check($mybb->input['my_post_key']);

This way the user will actually return an error on their action.

#14 Updated by Diogo Parrinha over 2 years ago

Tom Moore wrote:

The other way around lol. This:

[...]

This way the user will actually return an error on their action.

This is why I didn't do it like that:

Diogo Parrinha wrote:

By the way my solution is to check the post key in passive mode and only change the language if the key is valid. Otherwise do nothing. It must be done this way because the templates are not loaded at that time thus we can't show any errors.

#15 Updated by Tom Moore over 2 years ago

Templates will be loaded dynamically, no?

#16 Updated by Diogo Parrinha over 2 years ago

Not sure why but in this case it is not being loaded. They're only "cached" some lines below. I'm currently working on the mods site so can't dig into the issue.

#17 Updated by Tom Moore over 2 years ago

Strange. Don't worry about it then - just thought an error message would be better then them not changing the language.

#18 Updated by Jitendra Maharaj over 2 years ago

  • Status changed from Resolved to Closed
  • SQA assignments set to Jitendra Maharaj

Problem seems to be gone now...dunno if you guys wanna keep this open anymore

Also available in: Atom PDF