Mybb: XSS and $mybb->input in Templates

[euantorano]

I'd like to point out that MyBB templates should never use $mybb->input directly in the templates. This creates a situation where the data is not sanitized on the page. This opens up XSS where links to the page could insert malicious code including javascript to execute on the site including things like login popups which may mimic site login popup and then steal the credentials.

XSS is low-risk but still a risk and it's not the best practice to use unsanitized user input anywhere. But MyBB does this in a number of the default/master templates.

Most of the time the $mybb->input is used in case there is an error in form submission and the data remains in the form. That's convenient for the member but it's a risk as well. I suggest that MyBB does a template search and see which templates do this in order to decide if they want to sanitize the input or just remove the convenience.

Please review asap.

Original thread: XSS and $mybb->input in Templates

[euantorano]

@effone found 29 cases, so we should check to see if every one of those cases is cleaned before being passed to the template.

[effone]

Line numbers of mybb_theme.xml where occurrences found:
5282, 6861, 6868, 6912, 6918, 6924, 6930, 6936, 6953, 6969, 7150, 7201, 7905, 7909, 7913, 8159, 8161, 8167, 8179, 8639, 10292, 10916, 10921 (2), 10925, 10948, 14075, 14079, 14083

[Sama34]

How would you fix this, stop using $mybb->input altogether or just making sure they get cleaned ? For instance $mybb->input['bday3'] is clean but for $mybb->input['birthday_year'] we get a piece of code that makes me question, aren't numbers always used for years ?

    if(!$errors)
    {
        $mybb->input = array_merge($user, $mybb->input);
        $birthday = explode('-', $user['birthday']);
        if(!isset($birthday[1]))
        {
            $birthday[1] = '';
        }
        if(!isset($birthday[2]))
        {
            $birthday[2] = '';
        }
        list($mybb->input['birthday_day'], $mybb->input['birthday_month'], $mybb->input['birthday_year']) = $birthday;
    }
    else
    {
        $errors = inline_error($errors);
    }

    // Sanitize all input
    foreach(array('usertitle', 'website', 'icq', 'aim', 'yahoo', 'skype', 'google', 'signature', 'birthday_day', 'birthday_month', 'birthday_year') as $field)
    {
        $mybb->input[$field] = htmlspecialchars_uni($mybb->get_input($field));
    }

There seems to be some that indeed need updating tho.

[euantorano]

I think it might be better for clarity to stop using $mybb->input in templates entirely. Introducing variables for each one makes it easier to determine that things are cleaned properly.

On 17 Apr 2018, at 07:21, Omar Gonzalez [e-mail] wrote:

How would you fix this, stop using $mybb->input altogether or just making sure they get cleaned ? For instance $mybb->input['bday3'] is clean but for $mybb->input['birthday_year'] we get a piece of code that makes me question, aren't numbers always used for years ?

if(!$errors)
{
$mybb->input = array_merge($user, $mybb->input);
$birthday = explode('-', $user['birthday']);
if(!isset($birthday[1]))
{
$birthday[1] = '';
}
if(!isset($birthday[2]))
{
$birthday[2] = '';
}
list($mybb->input['birthday_day'], $mybb->input['birthday_month'], $mybb->input['birthday_year']) = $birthday;
}
else
{
$errors = inline_error($errors);
}

// Sanitize all input
foreach(array('usertitle', 'website', 'icq', 'aim', 'yahoo', 'skype', 'google', 'signature', 'birthday_day', 'birthday_month', 'birthday_year') as $field)
{
$mybb->input[$field] = htmlspecialchars_uni($mybb->get_input($field));
}
There seems to be some that indeed need updating tho.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.