Should not be using the PHP function setcookie directly in code instead use our function SEC_setCookie

[eSilverStrike]

We seem to use the php function setcookie everywhere in Geeklog and the core plugins. For example in the lib-comment library:

            setcookie($_CONF['cookie_anon_name'], $username, time() + 31536000,
                $_CONF['cookie_path'], $_CONF['cookiedomain'],
                $_CONF['cookiesecure']);

In reality we should update this code to use the function in lib-security so mistakes do not happen with the last 3 variables (which I found I made in one of my plugins with not including cookie secure from $_CONFIG). Here is the function, if the last 3 variables are not set then the defaults will be used:

function SEC_setCookie($name, $value, $expire = 0, $path = null, $domain = null, $secure = null)

So the above code should be changed to along with everywhere else:

SEC_setCookie($_CONF['cookie_anon_name'], $username, time() + 31536000);

[mystralkk]

Fixed with change set 405c38a. I replaced setcookie() with SEC_setCookie except in rescue.php.

[eSilverStrike]

If I remember with my quick search the other day the setcookie is used for the password when logging into rescue and it doesn't set secure. This generates a warning in browsers at the moment as eventually it will be required.

I think we will need to add

, $_CONF['cookie_path'], $_CONF['cookiedomain'], $_CONF['cookiesecure']

to the function call while setting that cookie and add cookiesecure as a field that can be changed by rescue.

[mystralkk]

I fixed the "rescue.php" as you had advised me to. Thanks!

[eSilverStrike]

Did another update to SEC_setCookie for setting cookies based on new browser warnings about samesite attribute not being set and the default changing from None to Lax (to prevent 3rd party websites from accessing these cookies by default). Had to update code based on PHP version as well (since below PHP 7.3 you have to set samesite by adding to the path variable).

Do you see a problem by default setting all our cookies to Lax (accessible by the geeklog site only)? I did update SEC_setCookie so developers could pass a samesite attribute if they wished to set it to None or Strict.

I checked the session class and when it sets the session cookie it already appears to handle the samesite attribute. I don't see anywhere else now in Geeklog that doesn't use SEC_setCookie.

[mystralkk]

Do you see a problem by default setting all our cookies to Lax (accessible by the geeklog site only)? I did update SEC_setCookie so developers could pass a samesite attribute if they wished to set it to None or Strict.

No. Your fix looks reasonable to me.