MyBB Community Forums

Full Version: Data race in rebuild_settings()
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
MyBB 1.8 has a race condition where if multiple admins update a setting, any setting, simultaneously then it could lead to settings.php getting corrupted. I spoke to Euan about it, and he suggested something like flock() or similar to solve the problem.

Here's the actual implementation of rebuild_settings():
function rebuild_settings()
{
	global $db, $mybb;
	if(!file_exists(MYBB_ROOT."inc/settings.php"))
	{
		$mode = "x";
	}
	else
	{
		$mode = "w";
	}
	$options = array(
		"order_by" => "title",
		"order_dir" => "ASC"
	);
	$query = $db->simple_select("settings", "value, name", "", $options);
	$settings = null;
	while($setting = $db->fetch_array($query))
	{
		$mybb->settings[$setting['name']] = $setting['value'];
		$setting['value'] = addcslashes($setting['value'], '\\"$');
		$settings .= "\$settings['{$setting['name']}'] = \"{$setting['value']}\";\n";
	}
	$settings = "<"."?php\n/*********************************\ \n  DO NOT EDIT THIS FILE, PLEASE USE\n  THE SETTINGS EDITOR\n\*********************************/\n\n$settings\n";
	$file = @fopen(MYBB_ROOT."inc/settings.php", $mode);
	@fwrite($file, $settings);
	@fclose($file);
	$GLOBALS['settings'] = &$mybb->settings;
}
flock() is the easiest solution for sure. If we were to use file_put_contents() rather than the raw fopen(), fwrite() etc, you can specify locking as a parameter (since PHP 5.1, so it's compatible).
Hi,

Thank you for your report. We have pushed this issue to our Github repository for further analysis where you can track our commits and progress with fixing this bug. Discussions regarding this bug may also take place there too.

Follow this link to visit the issue on Github: https://github.com/mybb/mybb/issues/2855

Thanks for contributing to MyBB!

Regards,
The MyBB Group