expressjs / expressjs/cookie-parser
Signed cookie value is assigning with false when cookie secret is updated
- Dominant language
- JavaScript
- Stars
- 2k
- Forks
- 234
- PR merge metrics
- No merged PRs in 30d
Description
### Issue in detail:
Let's say a signed cookie is created with `['abc']` secret using `cookie-parser` module and save on the browser's cookie storage. Later on, due to the reason of cookie secret compromise, the user needs to change the cookie secret for e.g., `['xyz']` on the express server.
Now, the server is re-started with the updated cookie secret `['xyz']` and the user tries to fire an HTTP request method via browser on the same domain on another API endpoint of the server. As the signed cookie created with `['abc']` secret already exists on the browser's cookie storage (Assume the cookie is not expired), the same signed cookie is included by the browser under the `req` object and express server receives the same under `req.headers.cookie`.
In this case, as the cookie secret is updated, the `cookie-parser` module's middleware function will not be able to parse that signed cookie. I have gone through `signedCookies(obj, secret){}` function defined under the same module and found that in this particular case, the function is populating `req.signedCookies` object with key `cookieName` and value `false`.
Since that signed cookie is unable to parse with the updated secret, the `signedCookies(obj, secret){}` function should not include this scenario-type signed cookies under `req.signedCookies` object as they become invalid post cookie secret updation.
### The bug is reproducible by the below steps included with screenshots:
1) Initialize `cookieParser()` with secret `'abc'` using
`const cookieParser = require('cookie-parser');`
`const express = require('express');`
`const app = express();`
`app.use(cookieParser('abc'));`
2) Create a signed cookie and return the response using
`return res.cookie('csrf', 'csrfToken', {
expires: new Date(Date.now() + 30 * 60 * 1000),
httpOnly: true,
signed: true
});`
3) Update and save new cookie secret `'xyz'` using
`app.use(cookieParser('xyz'));`
4) Restart the express server.
5) Use browser to send HTTP request on the same server's domain on another API endpoint of the server.
6) The server gets the browser request and the request goes through `cookieParser(req, res, next)` middleware of `cookie-parser` module.
7) Execution flow comes under the module function `signedCookies (obj, secret){}`, this function does not handle possible values of `var dec` for `undefined` and `false` returned by `signedCookie(val, secret){}` function.

8) In this context, `false` value is returned by `signedCookie(val, secret){}` function as `cookie-signature` module is unable to unsign that signed cookie with the updated secret and thus returns `false`. As the validation for `undefined` and `false` values are not implemented, the `req.signedCookies` object is populated with that signed cookie consequently having cookie name as `csrf` and cookie value as `false`.

### Fixing the bug:
Handle the validation of afore-mentioned `undefined` and `false` values in such a way that all signed cookies possessing the scenario described above can be ignored and should neither be included in `req.cookies` nor in `req.signedCookies` objects as they become invalid post cookie secret updation.


I have already worked to fix this bug and will soon create a pull request tagging this issue no. as reference.
@dougwilson Kindly check on my careful observations and provide your meaning inputs on the same. If my work looks correct and appreciable, please merge my pull request which I will be creating after posting this issue.
Contributor guide
Assessment
This issue has not been assessed yet.