MyBB Community Forums

I have been working on an edit of a popular plugin: Default Avatar. I really liked it, but I wanted my users to get a random avatar. So far I have a very scruffy version running, but I was unsure if it was secure.

This is my code; please would a more experienced developer have a look over it and see if it is secure?

<?php

/**
 * MyBB 1.6
 * Copyright 2010 MyBB Group, All Rights Reserved
 *
 * Default Avatar by Santiago Dimattia
 * http://www.teleportz.com.ar
 * 
 */

if(!defined("IN_MYBB"))
{
	die("Direct initialization of this file is not allowed.<br /><br />Please make sure IN_MYBB is defined.");
}

$plugins->add_hook("member_do_register_end", "default_avatar_onsignup");
$plugins->add_hook("usercp_do_avatar_end", "default_avatar_onchange");
$plugins->add_hook("admin_config_settings_start", "default_avatar_update_current_users");

/**
 * Plugin information
 *
 * @return bool
 */
function default_avatar_info()
{
	global $lang;

	$lang->load('default_avatar');
	 
	$data = array(
			"name" => $lang->default_avatar_plugin_name,
			"description" => $lang->default_avatar_plugin_description,
			"website" => "http://mods.mybboard.net/view/default-avatar/",
			"author" => "Santiago Dimattia",
			"authorsite" => "http://teleportz.com.ar",
			"version" => "1.0",
			"guid" => "389824f8f3c7b9056bed9dc3ac0330da",
			"compatibility" => "16*"
		);
	
	$plugin_status = default_avatar_is_installed();

	if($plugin_status)
	{
		$data['description'] .= $lang->sprintf($lang->default_avatar_plugin_description_link, 'index.php?module=config&amp;defaultavatar=update_current_users');
	}
	
	return $data;
}

/**
 * Install plugin
 *
 * @return bool
 */
function default_avatar_install()
{
	global $db, $lang;
	
	// Create settings
	$new_configs = array();
	
	$new_configs[] = array(
		'name' => 'default_avatar_url',
		'title' => $lang->default_avatar_setting_url,
		'description' => $lang->default_avatar_setting_url_description,
		'optionscode' => 'text',
		'value' => 'images/avatars/invalid_url.gif',
		'disporder' => 40,
		'gid' => '9'
	);
	
	$new_configs[] = array(
		'name' => 'default_avatar_width',
		'title' => $lang->default_avatar_setting_width,
		'description' => $lang->default_avatar_setting_width_description,
		'optionscode' => 'text',
		'value' => '84',
		'disporder' => 41,
		'gid' => '9'
	);
	
	$new_configs[] = array(
		'name' => 'default_avatar_height',
		'title' => $lang->default_avatar_setting_height,
		'description' => $lang->default_avatar_setting_height_description,
		'optionscode' => 'text',
		'value' => '84',
		'disporder' => 42,
		'gid' => '9'
	);
	
	foreach($new_configs as $key => $data)
	{
		$db->insert_query("settings", $data);
	}
	
	rebuild_settings();
	
	return TRUE;
}

/**
 * Uninstall plugin
 *
 * @return bool
 */
function default_avatar_uninstall()
{
	global $db;
	
	$db->delete_query('settings', 'name = "default_avatar_url" OR name = "default_avatar_width" OR name = "default_avatar_height"');

	rebuild_settings();
	
	return TRUE;
}

/**
 * Check if plugin is installed
 *
 * @return bool
 */
function default_avatar_is_installed()
{
	global $mybb;
	
	// If setting exists, plugin is installed
	if(isset($mybb->settings['default_avatar_url']))
	{
		return TRUE;
	}
	
	return FALSE;
}

/**
 * Activate plugin
 *
 * @return bool
 */
function default_avatar_activate()
{
	// Delete "Delete Avatar" button
	require MYBB_ROOT . '/inc/adminfunctions_templates.php';

	find_replace_templatesets('usercp_avatar', '#' . preg_quote('<input type="submit" class="button" name="remove" value="{$lang->remove_avatar}" />') . '#', '<!-- DeleteAvatarButton -->');
	
	return TRUE;
}

/**
 * Deactivate plugin
 *
 * @return bool
 */
function default_avatar_deactivate()
{
	// Add "Delete Avatar" button
	require MYBB_ROOT . '/inc/adminfunctions_templates.php';

	find_replace_templatesets('usercp_avatar', '#<!--\sDeleteAvatarButton\s--\>#is', '<input type="submit" class="button" name="remove" value="{$lang->remove_avatar}" />', 0);
	return TRUE;
}

/**
 * Set a default avatar to all users
 *
 * @return bool
 */
function default_avatar_update_current_users()
{
	global $mybb, $db, $lang;
	
	$plugin_status = default_avatar_is_installed();
	
	if($mybb->input['defaultavatar'] == 'update_current_users' && $plugin_status)
	{
		$lang->load('default_avatar');
		
		$set_avatar = get_default_avatar_data();
		$db->update_query('users', $set_avatar, 'avatar = "" OR avatar = "images/avatars/clear_avatar.gif"');
		
		flash_message($lang->default_avatar_message_all_users_updated, 'success');
		admin_redirect('index.php?module=config-plugins');
		
		return TRUE;
	}
	
	return FALSE;
}

/**
 * Set default avatar on signup
 *
 * @return bool
 */
function default_avatar_onsignup()
{
	global $mybb, $db, $user_info;
	
	$set_avatar = get_default_avatar_data();
	
	$db->update_query('users', $set_avatar, 'uid = ' . $user_info['uid']);
	
	return true;
}

/**
 * Set default avatar when the user delete his current avatar
 *
 * @return bool
 */
function default_avatar_onchange()
{
	global $mybb, $db;
	
	if($mybb->input['remove'] OR ($mybb->input['gallery'] && $mybb->input['avatar'] == 'clear_avatar.gif'))
	{
		$set_avatar = get_default_avatar_data();
		
		$db->update_query('users', $set_avatar, 'uid = ' . $mybb->user['uid']);
	}
	
	return true;
}

/**
 * Get default avatar data from MyBB Settings
 *
 * @return array Avatar data
 */


function get_default_avatar_data()
{
	global $mybb;
	
	function array_random($arr, $num = 1) {
    shuffle($arr);
    
    $r = array();
    for ($i = 0; $i < $num; $i++) {
        $r[] = $arr[$i];
    }
    return $num == 1 ? $r[0] : $r;
} 
$a = array("images/avatars/jack.jpg", "images/avatars/kronk.jpg", "images/avatars/mel.jpg","images/avatars/neoSpaz.jpg", "images/avatars/ninja.jpg", "images/avatars/zoe.jpg");
$defavatar = (array_random($a));
	
	$avatar = array(
		'avatar' => $defavatar,
		'avatardimensions' => $mybb->settings['default_avatar_width'] . '|' . $mybb->settings['default_avatar_height'],
		'avatartype' => 'remote'
	);
	
	return $avatar;
}

TO DO:

1. make a "query" that grabs all the avatars from the avatars folder and use that instead of a hardcoded array.
   
2. Tidy up the plugin inputs
   

I'm not 100% sure about your code since I'm not such a security expert but it should be fine as long as you don't insert without escaping data which comes from directly from the user.

Btw, some improvements:

$defavatar = (array_random($a));

You don't need to define a function for this, array_rand() already does the job.

$defavatar = array_rand($a);

I'd also check if the plugin is installed or not just once, before hooking, and therefore avoiding checking for it in every function.

if (default_avatar_is_installed()) {
    $plugins->add_hook("member_do_register_end", "default_avatar_onsignup");
    $plugins->add_hook("usercp_do_avatar_end", "default_avatar_onchange");
    $plugins->add_hook("admin_config_settings_start", "default_avatar_update_current_users");
}

Thank you Shade, but isnt an array shuffle "better" than rand? I will try it though as it will tidy up the code.

Good catch on the hooks, I shall do that

EDIT: Changing to array rand sort of failed. I think it is because array_rand is getting the key, whereas my function is getting the actual value from the array.

You'd just pick it up from the array then:

$key = array_rand($a);
$defavatar = $a[$key];

Would need a for each then right? Cos the key is a number, not a string... This is my worry on the security.

Nope. $a[$key] gives you the value associated with that key, which you obtain using array_rand() and it's not controlled by the user's inputs.

You should $db->escape_string the $lang->*setting* stuff, otherwise you'll get an SQL error when anyone uses ' in the language file.

Shade, that code is giving me the array position, not its value. This is because I am now trying to run a scandir on the avatar directory and that is returning an associative array.

I shall struggle on - very close now.

Ok then, array_rand() returns an array and not an integer. That's not what I did expect.

From the docs:

$defavatar = $a[mt_rand(0, count($a) - 1)];

This should be even faster.

Excellent; got it all working and the avatars are being pulled from the directory rather than being a hardcoded string