mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
API CHANGE Disallow methods/actions in RequestHandler->checkAccessAction() which are implemented on parent classes (e.g. ViewableData and Object), unless access is controlled through $allowed_actions. This limits information exposure from getters used in template contexts.
git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@102003 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
parent
a4c9707af2
commit
4c74f45361
@ -233,6 +233,7 @@ class RequestHandler extends ViewableData {
|
||||
* It will interrogate {@link self::$allowed_actions} to determine this.
|
||||
*/
|
||||
function checkAccessAction($action) {
|
||||
$actionOrigCasing = $action;
|
||||
$action = strtolower($action);
|
||||
$allowedActions = $this->allowedActions();
|
||||
|
||||
@ -262,13 +263,25 @@ class RequestHandler extends ViewableData {
|
||||
|
||||
// If we get here an the action is 'index', then it hasn't been specified, which means that
|
||||
// it should be allowed.
|
||||
if($action == 'index') return true;
|
||||
if($action == 'index' || empty($action)) return true;
|
||||
|
||||
if($allowedActions === null || !$this->uninherited('allowed_actions')) {
|
||||
// If no allowed_actions are provided, then we should only let through actions that aren't handled by magic methods
|
||||
// we test this by calling the unmagic method_exists and comparing it to the magic $this->hasMethod(). This will
|
||||
// still let through actions that are handled by templates.
|
||||
return method_exists($this, $action) || !$this->hasMethod($action);
|
||||
// we test this by calling the unmagic method_exists.
|
||||
if(method_exists($this, $action)) {
|
||||
// Disallow any methods which aren't defined on RequestHandler or subclasses
|
||||
// (e.g. ViewableData->getSecurityID())
|
||||
$r = new ReflectionClass(get_class($this));
|
||||
if($r->hasMethod($actionOrigCasing)) {
|
||||
$m = $r->getMethod($actionOrigCasing);
|
||||
return ($m && is_subclass_of($m->class, 'RequestHandler'));
|
||||
} else {
|
||||
throw new Exception("method_exists() true but ReflectionClass can't find method - PHP is b0kred");
|
||||
}
|
||||
} else if(!$this->hasMethod($action)){
|
||||
// Return true so that a template can handle this action
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
|
@ -122,6 +122,11 @@ class RequestHandlingTest extends SapphireTest {
|
||||
$this->assertEquals('This page does not exist.', $response->getBody());
|
||||
}
|
||||
|
||||
function testMethodsOnParentClassesOfRequestHandlerDeclined() {
|
||||
$response = Director::test('testGoodBase1/getSecurityID');
|
||||
$this->assertEquals(403, $response->getStatusCode());
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
Loading…
Reference in New Issue
Block a user