bug #23238 [Security] ensure the 'route' index is set before attempting to use it (gsdevme)
This PR was submitted for the 2.8 branch but it was merged into the 2.7 branch instead (closes #23238).
Discussion
----------
[Security] ensure the 'route' index is set before attempting to use it
| Q | A
| ------------- | ---
| Branch? | 2.8
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
```
// matching a request is more powerful than matching a URL path + context, so try that first
if ($this->urlMatcher instanceof RequestMatcherInterface) {
$parameters = $this->urlMatcher->matchRequest($request);
} else {
$parameters = $this->urlMatcher->match($request->getPathInfo());
}
return $path === $parameters['_route'];
```
Hi the issue here is the code is assuming a `_route` has been returned from the `match()` method.. however there is nothing to suggest that is always the case. For example if I just want to return a controller that is perhaps not added as an actual route I can & it works.. Although this will generate a notice warning.
**In terms of what happens if the `_route` is not defined should it return `false?` or actually perform a similar condition as `return $path === rawurldecode($request->getPathInfo());` **
I have an implementation of a router that is just returning a controller path and its arguments without a `_route` which works aside from this notice.
Commits
-------
7ae578cc1a
fix(security): ensure the 'route' index is set before attempting to use it
This commit is contained in:
commit
f4fffc082c
@ -108,7 +108,7 @@ class HttpUtils
|
|||||||
$parameters = $this->urlMatcher->match($request->getPathInfo());
|
$parameters = $this->urlMatcher->match($request->getPathInfo());
|
||||||
}
|
}
|
||||||
|
|
||||||
return $path === $parameters['_route'];
|
return isset($parameters['_route']) && $path === $parameters['_route'];
|
||||||
} catch (MethodNotAllowedException $e) {
|
} catch (MethodNotAllowedException $e) {
|
||||||
return false;
|
return false;
|
||||||
} catch (ResourceNotFoundException $e) {
|
} catch (ResourceNotFoundException $e) {
|
||||||
|
@ -221,6 +221,19 @@ class HttpUtilsTest extends TestCase
|
|||||||
$utils->checkRequestPath($this->getRequest(), 'foobar');
|
$utils->checkRequestPath($this->getRequest(), 'foobar');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testCheckPathWithoutRouteParam()
|
||||||
|
{
|
||||||
|
$urlMatcher = $this->getMockBuilder('Symfony\Component\Routing\Matcher\UrlMatcherInterface')->getMock();
|
||||||
|
$urlMatcher
|
||||||
|
->expects($this->any())
|
||||||
|
->method('match')
|
||||||
|
->willReturn(array('_controller' => 'PathController'))
|
||||||
|
;
|
||||||
|
|
||||||
|
$utils = new HttpUtils(null, $urlMatcher);
|
||||||
|
$this->assertFalse($utils->checkRequestPath($this->getRequest(), 'path/index.html'));
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @expectedException \InvalidArgumentException
|
* @expectedException \InvalidArgumentException
|
||||||
* @expectedExceptionMessage Matcher must either implement UrlMatcherInterface or RequestMatcherInterface
|
* @expectedExceptionMessage Matcher must either implement UrlMatcherInterface or RequestMatcherInterface
|
||||||
|
Reference in New Issue
Block a user