CodeIgniter Forums

Full Version: Once the model is loaded by the controller all model methods are exposed to view
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
If in Controller I do this:

Code:
$this->load->model('mymodel');
//...do some stuff with mymodel...

//calling the view
$vars['somedata'] = 'mydata';
$this->load->view('someview', $vars);


Inside the View someview.php you can do this
Code:
$this->mymodel->someMethod();

Isn't this a security threat? Huh

The View should only be able:
  • to see the data passed by the Controller
  • and to load others Views
but the View should NOT be allowed to call directly methods of any model.
This is not a security threat, it is the "CodeIgniter 3 way".

$this->load->view(...) builds an "include" argument, and "includes" it, which is why all of the controller properties (including models) are accessible inside the view.

There are measures to prevent it being a threat ... .htaccess rules inside the application & system folders, as well as the "defined('BASEPATH') OR exit('No direct script access allowed');" snippet at the beginning of each file.
We also suggest creating a "public" folder, and moving index.php there, for improved security in the case of a mis-configured app.

If a *developer* so chooses, yes they can access models directly. On the other hand, if they only rely on data passed as a parameter to the load->view, then they have better separation of concerns.

By the way, our website and repository are quite clear on the procedure for reporting security concerns - through an email to our security team, or through a report on hackerone.com, *not* through a message on a public forum!
(12-08-2018, 07:37 PM)ciadmin Wrote: [ -> ]By the way, our website and repository are quite clear on the procedure for reporting security concerns - through an email to our security team, or through a report on hackerone.com, *not* through a message on a public forum!

Oh, sorry, I thought those ways were only to be used if someone spots out a security bug/threat.
My one is just a concern, in order to better understand how CodeIgniter works and what's the best practice in this case.

(12-08-2018, 07:37 PM)ciadmin Wrote: [ -> ]This is not a security threat, it is the "CodeIgniter 3 way".

$this->load->view(...) builds an "include" argument, and "includes" it, which is why all of the controller properties (including models) are accessible inside the view.

There are measures to prevent it being a threat ... .htaccess rules inside the application & system folders, as well as the "defined('BASEPATH') OR exit('No direct script access allowed');" snippet at the beginning of each file.
We also suggest creating a "public" folder, and moving index.php there, for improved security in the case of a mis-configured app.

If a *developer* so chooses, yes they can access models directly. On the other hand, if they only rely on data passed as a parameter to the load->view, then they have better separation of concerns.

Ok, let me explain what I meant.

Usually we give outside our View files (along with css, js etc.) to an external developer/company that updates them and keep them beautiful.

We do our best effort to avoid that PHP code inserted by mistake in any View file can access some Models or anything else.

But in order to create the View and pass data to it we need the Controller to load and access the Model with the typical

Code:
//in the Controller constructor we do
$this->load->model('invoices');

Code:
//Then, in another function of the same Controller we show the view
$vars['somedata'] = 'mydata';
$this->load->view('someview', $vars);

Let's say we give the View 'someview.php' to an external developer that is supposed to only make the View more beautiful, but he by mistake adds into the view a piece of code that deletes an entire table.

Code:
<php $this->invoices->deleteAll(); ?>

So once the external developer give the View file back to us, we basically can't put the Views directly in production unless we thoroughly check each line of the View code to make sure the external developer did not add by mistake dangerous PHP code.

If we could unload the Models before loading views, the line of code shown above would not work, hence we could put in production the new View files without the need to thoroughly check each line of code.
So is there a way to unload Models before loading views? Huh

Is it a best practice to unload Model before loading View? Confused
I believe this bit of code is what controls loading up those models to be accessible in a view:

PHP Code:
// This allows anything loaded using $this->load (views, files, etc.)
// to become accessible from within the Controller and Model functions.

$_ci_CI =& get_instance();
foreach (
get_object_vars($_ci_CI) as $_ci_key => $_ci_var)
{
    if ( ! isset(
$this->$_ci_key))
    {
        
$this->$_ci_key =& $_ci_CI->$_ci_key;
    }


It is found in system/ci/core/loader.php protected function _ci_load, at least in my version of CI.

Since it is used by several loaders you would want to include another variable flag in the calling function: 
PHP Code:
public function view($view$vars = array(), $return FALSE)
{
    return 
$this->_ci_load(array('_ci_view' => $view'_ci_vars' => $this->_ci_object_to_array($vars), '_ci_return' => $return), $MY_NEW_BOOL_FLAG);
    
//You could also add '_ci_prevent_model_load' => true to the data package and check for that in _ci_load, that's cleaner and follows their convention


Add that flag to the _ci_load() function, and then when it's true you could probably do something like check if(is_subclass_of($_ci_CI->$_ci_key, 'CI_Model')) then don't load it.

The function opens with:
PHP Code:
// Set the default data variables
foreach (array('_ci_view''_ci_vars''_ci_path''_ci_return') as $_ci_val)
{
    $
$_ci_val = ( ! isset($_ci_data[$_ci_val])) ? FALSE $_ci_data[$_ci_val];

so it seems like your $vars data would still be loaded up.

I haven't tried this, but if it doesn't work just like that I think it would put you on the path to getting things to work as you like.
As in normal circumstances you won't execute outputted strings, so users won't be able to add comments like '<?php $this->model->naughtyCall(); ?>'.

That leaves just developers, who have access to PHP code and would be able to write in these calls.

If you use Git, it's fairly easy to browse through changes and verify what they have done.

But if you are afraid they would do stuff like that and you don't trust them to supply code, you might need new more trustworthy developers.
Your handing over javaScript and not going over every inch of what they return? Seems just as dangerous as trusting the view code to them.