Potentially unsafe global request data is accessible in Action::initialize() and View::initialize() and others

[david]

Not sure if we regard this a bug or not.

My suggestion is to lock the request for both, and also during other calls like isSecure() and getCredentials().

The reason why I think this needs fixing is that if we encourage people to change the output type in a view's initialize method based on request data, then there shouldn't be a way to access potentially insecure request data there.

I don't think there is a use case for accessing the request data in Action::initialize(), and most people probably did the right thing and used the container's request data in View::initialize() anyway, so there shouldn't be much BC breakage (we could label this a security fix and just forget about it, or make this "hardened" mode configurable).

Opinions please?

[MugeSo]

I think it will be solved to run validaters in ExecutionContainer::execute(). Or create ValidationFilter? which runs Validaters and register it before SecurityFilter?.

[MugeSo]

This patch enables to validation anywhere in ActionFilterChain?

[MugeSo]

change in ValidationRunTimeCache?.patch is controller/ CHG:

AgaviExecutionContaioner?

ADD: validateRequestData: validate container's requestData

filter/ CHG:

AgaviExecutionFilter?

CHG: runAction: replace validation to using AgaviExecutionContaioner::validateRequestData

Example: class SomeAction? extends AgaviAction? {

public getCredentials() {

$container = $this->getContainer(); if($container->validateRequestData()) {

// validated, you can use requestData safe.

} else {

// not validated!! requestData is unsafe.

}

}

}

[MugeSo]

replace indent-space to tab.

[MugeSo]

another solution. (add methods for safe handling request data)

[MugeSo]

add parameter "AgaviRequestDataHolder? $rd" to request data safe methods.

[MugeSo]

Example for addActionMethodsForUseRequestData.2.patch

class SomeAbstructAction extends AgaviAction
{
    /**
     * @var	  SomeModel a model instance
     */
    protected $myModel = null;

    /**
     * initialize $myModel, which is used by child classes.
     */
    public function initializeWithRequestData(AgaviRequestDataHolder $rd)
    {
	$name = $rd->getParameter('name');
	$this->myModel = $this->getContext()->getModel("SomeModel", null, array('name'=>$name));
    }

    /**
     * answer this action is secure or not, acording to request data
     */
    public function isSecureWithRequestData(AgaviRequestDataHolder $rd)
    {
	return $rd->getParameter('subject')=='secret-subject';
    }

    /**
     * answer this action is secure or not if validation fails
     */
    public function isSecure()
    {
	return false;
    }
    
    /**
     * answer credentials, acording to request data
     */
    public function getCredentialsWithRequestData(AgaviRequestDataHolder $rd)
    {
	return array('credential-' . $rd->getParameter('item_id'));
    }

    /**
     * answer credentials if validation fails
     */
    public function getCredentials()
    {
	return null;
    }
}

[david]

(In [2108]) Controller now gives containers a copy of the global request data holder. This means "request_lock_barf" is no longer used; you can create containers in your code and they will have the actual request data in them. Rejoice. Refs #544. Also cleaned up the locking code and made it more secure. And did related bug fixes.

[david]

(In [2114]) Do not store global request data copy in serialized execution containers, and restore current global request data if possible on wakeup. Refs #544

[david]

(In [2115]) Made global request data copy unavailable from the outside of execution containers until cloned in execute(), refs #544

[david]

(In [2117]) Fixed global request data being accessible in View::initialize() (and in the constructor), closes #544 (best we can do with our current architecture)