Codeigniter cookies too strict

[boblennes]

I had an issue last night with one of my Bonfire apps running alongside an ecommerce app. Apparently cookie names can have additional characters now like period, ']', '[', '%' and '@' The problem cookie for me was from Adobe (thanks Adobe! ha..). The symptom is the page wont render, only there is a message "Disallowed key characters".

So the fix is in the input helper but I wasn't sure where it belonged in Bonfire. Here's the code:

<?php

class MY_Input extends CI_Input {

    /**
     * Clean Keys
     *
     * This is a helper function. To prevent malicious users
     * from trying to exploit keys we make sure that keys are
     * only named with alpha-numeric text and a few other items.
     * 
     * Extended to allow: 
     *      '.' (dot), '[' (open bracket), ']' (close bracket), '@' ("at"), '%' (percent)
     * 
     * @access  private
     * @param   string
     * @return  string
     */
    function _clean_input_keys($str) {
        if (!preg_match("/^[A-Za-z0-9:_\/-\[\]@%\.]+$/i", $str)) {
            /**
             * Check for Development environment - Non-descriptive 
             * error so show me the string that caused the problem 
             */
            if (getenv('ENVIRONMENT') && getenv('ENVIRONMENT') == 'DEVELOPMENT')
                var_dump($str);
            exit('Disallowed Key Characters.');
        }

        // Clean UTF-8 if supported
        if (UTF8_ENABLED === TRUE) {
            $str = $this->uni->clean_string($str);
        }

        return $str;
    }

}

[mwhitneysdsu]

Unfortunately, the function is used for the $_GET and $_POST arrays, too, so it would probably be better to override _sanitize_globals() and pass off the cleaning of the cookie's keys to a different method.

Another option would be to add a check to sanitize_globals() for something which would identify the failing cookie properly and skip the call to clean the key/data for that particular cookie (since there's already logic in place to do this for the session cookie if it is encrypted, this may be the easiest, and safest, option).

[boblennes]

I looked at the latest Codeigniter dev and it may be addressed by ignoring bad keys rather than dying:

        // Clean $_COOKIE Data
        if (is_array($_COOKIE) && count($_COOKIE) > 0)
        {
            // Also get rid of specially treated cookies that might be set by a server
            // or silly application, that are of no use to a CI application anyway
            // but that when present will trip our 'Disallowed Key Characters' alarm
            // http://www.ietf.org/rfc/rfc2109.txt
            // note that the key names below are single quoted strings, and are not PHP variables
            unset(
                $_COOKIE['$Version'],
                $_COOKIE['$Path'],
                $_COOKIE['$Domain']
            );

            foreach ($_COOKIE as $key => $val)
            {
                if (($cookie_key = $this->_clean_input_keys($key)) !== FALSE)
                {
                    $_COOKIE[$cookie_key] = $this->_clean_input_data($val);
                }
                else
                {
                    unset($_COOKIE[$key]);
                }
            }
        }

and

    /**
     * Clean Keys
     *
     * Internal method that helps to prevent malicious users
     * from trying to exploit keys we make sure that keys are
     * only named with alpha-numeric text and a few other items.
     *
     * @param   string  $str    Input string
     * @param   string  $fatal  Whether to terminate script exection
     *              or to return FALSE if an invalid
     *              key is encountered
     * @return  string|bool
     */
    protected function _clean_input_keys($str, $fatal = TRUE)
    {
        if ( ! preg_match('/^[a-z0-9:_\/|-]+$/i', $str))
        {
            if ($fatal === TRUE)
            {
                return FALSE;
            }
            else
            {
                set_status_header(503);
                echo 'Disallowed Key Characters.';
                exit(7); // EXIT_USER_INPUT
            }
        }

        // Clean UTF-8 if supported
        if (UTF8_ENABLED === TRUE)
        {
            return $this->uni->clean_string($str);
        }

        return $str;
    }

[mwhitneysdsu]

You could try something like the following, but I haven't tested to determine whether anything else would need to be updated to use the CI 3 methods correctly/safely.

https://gist.github.com/mwhitneysdsu/19c7bd5fb1f9192e93f7#file-bf_input-php

This file could be placed in /bonfire/core/ to replace the methods defined in /bonfire/codeigniter/core/Input.php.

Note that I had to change the visibility of the methods and change one function call to make it compatible with CI2.x, but this still doesn't guarantee that everything is functioning as it should. For instance, much of the Security code has changed significantly in CI3, so the Input class may be assuming that the Security class will be more aggressive in some areas.

[boblennes]

This seems to be working quite well.