Unrecognized source in CodeIgniter framework

AirSkye commented 1 year ago

This is a modified code from a real project (this is the CodeIgniter framework in php), there is an eval function call in the search method of the Book class (this is the controller in MVC), after loading the CI_Input class by including the input.php file , in the search method, call the get_post method of the CI_Input class to get the get parameter key. I added the annotation @psalm-taint-source input to the get_post, which can actually achieve the effect of command execution, but psalm did not detect it.

This is Book.php

<?php
include "./input.php";
class Book {
    public $input;
    public function __construct(){
        $this->input = new CI_Input();
    }

    public function search($key='') {

        $key = $this->input->get_post('key');
        eval($key);
    }
}
$a= new Book();
$a->search();

This is input.php

<?php
/**
 * CodeIgniter
 *
 * An open source application development framework for PHP
 *
 * This content is released under the MIT License (MIT)
 *
 * Copyright (c) 2014 - 2019, British Columbia Institute of Technology
 *
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 * THE SOFTWARE.
 *
 * @package CodeIgniter
 * @author  EllisLab Dev Team
 * @copyright   Copyright (c) 2008 - 2014, EllisLab, Inc. (https://ellislab.com/)
 * @copyright   Copyright (c) 2014 - 2019, British Columbia Institute of Technology (https://bcit.ca/)
 * @license https://opensource.org/licenses/MIT MIT License
 * @link    https://codeigniter.com
 * @since   Version 1.0.0
 * @filesource
 */

/**
 * Input Class
 *
 * Pre-processes global input data for security
 *
 * @package     CodeIgniter
 * @subpackage  Libraries
 * @category    Input
 * @author      EllisLab Dev Team
 * @link        https://codeigniter.com/user_guide/libraries/input.html
 */
class CI_Input {

    /**
     * IP address of the current user
     *
     * @var string
     */
    protected $ip_address = FALSE;

    /**
     * Allow GET array flag
     *
     * If set to FALSE, then $_GET will be set to an empty array.
     *
     * @var bool
     */
    protected $_allow_get_array = TRUE;

    /**
     * Standardize new lines flag
     *
     * If set to TRUE, then newlines are standardized.
     *
     * @var bool
     */
    protected $_standardize_newlines;

    /**
     * Enable XSS flag
     *
     * Determines whether the XSS filter is always active when
     * GET, POST or COOKIE data is encountered.
     * Set automatically based on config setting.
     *
     * @var bool
     */
    protected $_enable_xss = FALSE;

    /**
     * Enable CSRF flag
     *
     * Enables a CSRF cookie token to be set.
     * Set automatically based on config setting.
     *
     * @var bool
     */
    protected $_enable_csrf = FALSE;

    /**
     * List of all HTTP request headers
     *
     * @var array
     */
    protected $headers = array();

    /**
     * Raw input stream data
     *
     * Holds a cache of php://input contents
     *
     * @var string
     */
    protected $_raw_input_stream;

    /**
     * Parsed input stream data
     *
     * Parsed from php://input at runtime
     *
     * @see CI_Input::input_stream()
     * @var array
     */
    protected $_input_stream;

    protected $security;
    protected $uni;
    public $input;
    // --------------------------------------------------------------------

    /**
     * Class constructor
     *
     * Determines whether to globally enable the XSS processing
     * and whether to allow the $_GET array.
     *
     * @return  void
     */
    public function __construct()
    {
    }

    // --------------------------------------------------------------------

    /**
     * Fetch from array
     *
     * Internal method used to retrieve values from global arrays.
     *
     * @param   array   &$array     $_GET, $_POST, $_COOKIE, $_SERVER, etc.
     * @param   mixed   $index      Index for item to be fetched from $array
     * @param   bool    $xss_clean  Whether to apply XSS filtering
     * @return  mixed
     */
    protected function _fetch_from_array(&$array, $index = NULL, $xss_clean = NULL)
    {
        is_bool($xss_clean) OR $xss_clean = $this->_enable_xss;

        // If $index is NULL, it means that the whole $array is requested
        isset($index) OR $index = array_keys($array);

        // allow fetching multiple keys at once
        if (is_array($index))
        {
            $output = array();
            foreach ($index as $key)
            {
                $output[$key] = $this->_fetch_from_array($array, $key, $xss_clean);
            }

            return $output;
        }

        if (isset($array[$index]))
        {
            $value = $array[$index];
        }
        elseif (($count = preg_match_all('/(?:^[^\[]+)|\[[^]]*\]/', $index, $matches)) > 1) // Does the index contain array notation
        {
            $value = $array;
            for ($i = 0; $i < $count; $i++)
            {
                $key = trim($matches[0][$i], '[]');
                if ($key === '') // Empty notation will return the value as array
                {
                    break;
                }

                if (isset($value[$key]))
                {
                    $value = $value[$key];
                }
                else
                {
                    return NULL;
                }
            }
        }
        else
        {
            return NULL;
        }

        return $value;
    }

    // --------------------------------------------------------------------

    /**
     * Fetch an item from the GET array
     *
     * @param   mixed   $index      Index for item to be fetched from $_GET
     * @param   bool    $xss_clean  Whether to apply XSS filtering
     * @return  mixed
     */
    public function get($index = NULL, $xss_clean = NULL)
    {
        return $this->_fetch_from_array($_GET, $index, $xss_clean);
    }

    // --------------------------------------------------------------------

    /**
     * Fetch an item from the POST array
     *
     * @param   mixed   $index      Index for item to be fetched from $_POST
     * @param   bool    $xss_clean  Whether to apply XSS filtering
     * @return  mixed
     */
    public function post($index = NULL, $xss_clean = NULL)
    {
        return $this->_fetch_from_array($_POST, $index, $xss_clean);
    }

    // --------------------------------------------------------------------

    /**
     * Fetch an item from POST data with fallback to GET
     *
     * @param   string  $index      Index for item to be fetched from $_POST or $_GET
     * @param   bool    $xss_clean  Whether to apply XSS filtering
     * @return  mixed
     */
    public function post_get($index, $xss_clean = NULL)
    {
        return isset($_POST[$index])
            ? $this->post($index, $xss_clean)
            : $this->get($index, $xss_clean);
    }

    /**
     * @psalm-taint-source input
     */
    public function get_post($index, $xss_clean = NULL)
    {

        return isset($_GET[$index])
            ? $this->get($index, $xss_clean)
            : $this->post($index, $xss_clean);
    }

    // --------------------------------------------------------------------

    /**
     * Fetch an item from the COOKIE array
     *
     * @param   mixed   $index      Index for item to be fetched from $_COOKIE
     * @param   bool    $xss_clean  Whether to apply XSS filtering
     * @return  mixed
     */
    public function cookie($index = NULL, $xss_clean = NULL)
    {
        return $this->_fetch_from_array($_COOKIE, $index, $xss_clean);
    }

    // --------------------------------------------------------------------

    /**
     * Fetch an item from the SERVER array
     *
     * @param   mixed   $index      Index for item to be fetched from $_SERVER
     * @param   bool    $xss_clean  Whether to apply XSS filtering
     * @return  mixed
     */
    public function server($index, $xss_clean = NULL)
    {
        return $this->_fetch_from_array($_SERVER, $index, $xss_clean);
    }

    // ------------------------------------------------------------------------

    /**
     * Fetch an item from the php://input stream
     *
     * Useful when you need to access PUT, DELETE or PATCH request data.
     *
     * @param   string  $index      Index for item to be fetched
     * @param   bool    $xss_clean  Whether to apply XSS filtering
     * @return  mixed
     */
    public function input_stream($index = NULL, $xss_clean = NULL)
    {
        // Prior to PHP 5.6, the input stream can only be read once,
        // so we'll need to check if we have already done that first.
        if ( ! is_array($this->_input_stream))
        {
            // $this->raw_input_stream will trigger __get().
            parse_str($this->raw_input_stream, $this->_input_stream);
            is_array($this->_input_stream) OR $this->_input_stream = array();
        }

        return $this->_fetch_from_array($this->_input_stream, $index, $xss_clean);
    }

    // ------------------------------------------------------------------------

    /**
     * Set cookie
     *
     * Accepts an arbitrary number of parameters (up to 7) or an associative
     * array in the first parameter containing all the values.
     *
     * @param   string|mixed[]  $name       Cookie name or an array containing parameters
     * @param   string      $value      Cookie value
     * @param   int     $expire     Cookie expiration time in seconds
     * @param   string      $domain     Cookie domain (e.g.: '.yourdomain.com')
     * @param   string      $path       Cookie path (default: '/')
     * @param   string      $prefix     Cookie name prefix
     * @param   bool        $secure     Whether to only transfer cookies via SSL
     * @param   bool        $httponly   Whether to only makes the cookie accessible via HTTP (no javascript)
     * @return  void
     */

    // --------------------------------------------------------------------

    // ------------------------------------------------------------------------

    /**
     * Magic __get()
     *
     * Allows read access to protected properties
     *
     * @param   string  $name
     * @return  mixed
     */
    public function __get($name)
    {
        if ($name === 'raw_input_stream')
        {
            isset($this->_raw_input_stream) OR $this->_raw_input_stream = file_get_contents('php://input');
            return $this->_raw_input_stream;
        }
        elseif ($name === 'ip_address')
        {
            return $this->ip_address;
        }
    }

}

This is the execution result This is the ../vendor/bin/psalm --taint-analysis result

php == 7.4.30 psalm == 5.12.0@f90118cdeacd0088e7215e64c0c99ceca819e176

psalm-github-bot[bot] commented 1 year ago

Hey @AirSkye, can you reproduce the issue on https://psalm.dev ?

orklah commented 1 year ago

Well this should work. At least it does when I make a snippet without CodeIgniter

can you add a /* @psalm-trace $this->input / and make sure you display infos (--show-info should be enough)

AirSkye commented 1 year ago

I simplified the code and found that psalm could not find this vulnerability，Using --show-info returns no results https://psalm.dev/r/feac64ed05

<?php // --taint-analysis

class CI_Input {
    /** @psalm-taint-source input */
    public function get_post($x){
        return $_GET[$x];
    }
}

/** @psalm-trace $this->input */
class Book {

    public $input;
    public function __construct(){
        $this->input = new CI_Input();
    }

    public function search($key='') {
        $key = $this->input->get_post('key');
        eval($key);
    }
}
$a= new Book();
$a->search();

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/feac64ed05

```php input */ class Book { public $input; public function __construct(){ $this->input = new CI_Input(); } public function search($key='') { $key = $this->input->get_post('key'); eval($key); } } $a= new Book(); $a->search(); ``` ``` Psalm output (using commit a0a9c27): No issues! ```

orklah commented 1 year ago

You should make sure to fix important issues first, Taint analysis can only work properly if Psalm is able to infer your code correctly.

In this case, you're missing a property type: https://psalm.dev/r/a66f34a473

Psalm was not able to see that $this->input was a CI_input so it couldn't find your taint annotation

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/a66f34a473

```php input */ class Book { /** @var CI_Input */ public $input; public function __construct(){ $this->input = new CI_Input(); } public function search($key='') { $key = $this->input->get_post('key'); eval($key); } } $a= new Book(); $a->search(); ``` ``` Psalm output (using commit a0a9c27): ERROR: TaintedEval - 20:14 - Detected tainted code passed to eval or similar ```

vimeo / psalm

Unrecognized source in CodeIgniter framework #9952