Skip to content

Commit

Permalink
Fixes #3215 Add SameSite=lax flag to authentication cookies (#3219)
Browse files Browse the repository at this point in the history
* samesite attribute for authentication cookies

* added ACP settings

* update script changes

* update script changes

* corrections
  • Loading branch information
effone authored and dvz committed May 18, 2018
1 parent 3ed9844 commit 23df8fb
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 13 deletions.
2 changes: 1 addition & 1 deletion admin/index.php
Expand Up @@ -299,7 +299,7 @@
$db->update_query("adminoptions", array("loginattempts" => 0, "loginlockoutexpiry" => 0), "uid='{$mybb->user['uid']}'");
}

my_setcookie("adminsid", $sid, '', true);
my_setcookie("adminsid", $sid, '', true, "lax");
my_setcookie('acploginattempts', 0);
$post_verify = false;

Expand Down
2 changes: 1 addition & 1 deletion admin/modules/config/settings.php
Expand Up @@ -1082,7 +1082,7 @@
{
my_unsetcookie("adminsid");
$mybb->settings['cookieprefix'] = $mybb->input['upsetting']['cookieprefix'];
my_setcookie("adminsid", $admin_session['sid'], '', true);
my_setcookie("adminsid", $admin_session['sid'], '', true, "lax");
}

if(isset($mybb->input['upsetting']['statstopreferrer']) && $mybb->input['upsetting']['statstopreferrer'] != $mybb->settings['statstopreferrer'])
Expand Down
2 changes: 1 addition & 1 deletion inc/datahandlers/login.php
Expand Up @@ -326,7 +326,7 @@ function complete_login()
$remember = -1;
}

my_setcookie("mybbuser", $user['uid']."_".$user['loginkey'], $remember, true);
my_setcookie("mybbuser", $user['uid']."_".$user['loginkey'], $remember, true, "lax");

if($this->captcha !== false)
{
Expand Down
13 changes: 12 additions & 1 deletion inc/functions.php
Expand Up @@ -1859,8 +1859,9 @@ function get_post_icons()
* @param string $value The cookie value.
* @param int|string $expires The timestamp of the expiry date.
* @param boolean $httponly True if setting a HttpOnly cookie (supported by the majority of web browsers)
* @param string $samesite The samesite attribute to prevent CSRF.
*/
function my_setcookie($name, $value="", $expires="", $httponly=false)
function my_setcookie($name, $value="", $expires="", $httponly=false, $samesite="")
{
global $mybb;

Expand Down Expand Up @@ -1909,6 +1910,16 @@ function my_setcookie($name, $value="", $expires="", $httponly=false)
$cookie .= "; HttpOnly";
}

if($samesite != "" && $mybb->settings['cookiesamesiteflag'])
{
$samesite = strtolower($samesite);

if($samesite == "lax" || $samesite == "strict")
{
$cookie .= "; SameSite=".$samesite;
}
}

if($mybb->settings['cookiesecureflag'])
{
$cookie .= "; Secure";
Expand Down
2 changes: 1 addition & 1 deletion install/index.php
Expand Up @@ -2367,7 +2367,7 @@ function install_done()
// Automatic Login
my_unsetcookie("sid");
my_unsetcookie("mybbuser");
my_setcookie('mybbuser', $uid.'_'.$loginkey, null, true);
my_setcookie('mybbuser', $uid.'_'.$loginkey, null, true, "lax");
ob_end_flush();

// Make fulltext columns if supported
Expand Down
16 changes: 12 additions & 4 deletions install/resources/settings.xml
Expand Up @@ -107,34 +107,42 @@
<settingvalue><![CDATA[]]></settingvalue>
<isdefault>1</isdefault>
</setting>
<setting name="cookiesamesiteflag">
<title>SameSite Cookie Flag</title>
<description><![CDATA[Authentication cookies will carry the SameSite flag to prevent CSRF attacks. Keep this disabled if you expect cross-origin POST requests.]]></description>
<disporder>12</disporder>
<optionscode><![CDATA[yesno]]></optionscode>
<settingvalue><![CDATA[1]]></settingvalue>
<isdefault>1</isdefault>
</setting>
<setting name="cookiesecureflag">
<title>Secure Cookie Flag</title>
<description><![CDATA[Cookies can be set with the Secure flag to prevent them from being sent over unencrypted connections. You should enable this setting only if your forum works correctly under HTTPS.]]></description>
<disporder>12</disporder>
<disporder>13</disporder>
<optionscode><![CDATA[yesno]]></optionscode>
<settingvalue><![CDATA[0]]></settingvalue>
<isdefault>1</isdefault>
</setting>
<setting name="showvernum">
<title>Show Version Numbers</title>
<description><![CDATA[Allows you to turn off the public display of version numbers in MyBB.]]></description>
<disporder>13</disporder>
<disporder>14</disporder>
<optionscode><![CDATA[onoff]]></optionscode>
<settingvalue><![CDATA[0]]></settingvalue>
<isdefault>1</isdefault>
</setting>
<setting name="mailingaddress">
<title>Mailing Address</title>
<description><![CDATA[If you have a mailing address, enter it here. This is shown on the COPPA compliance form.]]></description>
<disporder>14</disporder>
<disporder>15</disporder>
<optionscode><![CDATA[textarea]]></optionscode>
<settingvalue><![CDATA[]]></settingvalue>
<isdefault>1</isdefault>
</setting>
<setting name="faxno">
<title>Contact Fax No</title>
<description><![CDATA[If you have a fax number, enter it here. This is shown on the COPPA compliance form.]]></description>
<disporder>15</disporder>
<disporder>16</disporder>
<optionscode><![CDATA[text]]></optionscode>
<settingvalue><![CDATA[]]></settingvalue>
<isdefault>1</isdefault>
Expand Down
2 changes: 1 addition & 1 deletion install/resources/upgrade43.php
Expand Up @@ -42,7 +42,7 @@ function upgrade43_dbchanges()
$db->drop_column('users', 'aim');
}
$db->delete_query("settings", "name='allowaimfield'");

$cache->delete("mybb_credits");

$output->print_contents("<p>Click next to continue with the upgrade process.</p>");
Expand Down
2 changes: 1 addition & 1 deletion install/upgrade.php
Expand Up @@ -207,7 +207,7 @@
}
}

my_setcookie("mybbuser", $user['uid']."_".$user['loginkey'], null, true);
my_setcookie("mybbuser", $user['uid']."_".$user['loginkey'], null, true, "lax");

header("Location: ./upgrade.php");
}
Expand Down
2 changes: 1 addition & 1 deletion member.php
Expand Up @@ -391,7 +391,7 @@
if($mybb->settings['regtype'] != "randompass" && !isset($mybb->cookies['coppauser']))
{
// Log them in
my_setcookie("mybbuser", $user_info['uid']."_".$user_info['loginkey'], null, true);
my_setcookie("mybbuser", $user_info['uid']."_".$user_info['loginkey'], null, true, "lax");
}

if(isset($mybb->cookies['coppauser']))
Expand Down
2 changes: 1 addition & 1 deletion usercp.php
Expand Up @@ -1292,7 +1292,7 @@
else
{
$userhandler->update_user();
my_setcookie("mybbuser", $mybb->user['uid']."_".$userhandler->data['loginkey'], null, true);
my_setcookie("mybbuser", $mybb->user['uid']."_".$userhandler->data['loginkey'], null, true, "lax");

// Notify the user by email that their password has been changed
$mail_message = $lang->sprintf($lang->email_changepassword, $mybb->user['username'], $mybb->user['email'], $mybb->settings['bbname'], $mybb->settings['bburl']);
Expand Down

0 comments on commit 23df8fb

Please sign in to comment.