Closed ataube closed 1 week ago
worth noting, we recently added a feature to datastore sessions related to session expiry:
https://github.com/googleapis/nodejs-datastore-session/pull/134
Not sure if we could do something similar with firestore; does firestore allow an expiry to be set on specific keys at the database level?
I'm flagging that this is a security vulnerability. I've reported this to Google Security and was instructed to publicly disclose the vulnerability here as it doesn't meet their threshold for internal handling. 🤷🏽
Not only are old sessions not cleaned up, they remain active. The cryptographic signature and other handling do not take timestamps into account, so while the cookie expires client side, the session is permanent on the server. When using this to track an authenticated session (username+password), you may expect that blocking login would prevent further access (after cookies expire) but the session is still active.
A threat vector here is fired employees. An employee could save their session cookie, alter the expiration time, and continue to access company resources without needing to use their (now disabled) username and password. Cookie theft is also a concern, but requires a separate attack. If a login is ever stolen you may expect that you can change the password and wait for the cookie to expire, but the attacker will still have access (you should manually delete the session from the store).
The lack of session expiry handling appears unique among the other express-sessions stores (I've only audited the ones that appear to be somewhat used), so this may surprise developers who've used express-sessions with other stores.
I strongly recommend that this project should enforce session expiration, consistent to other express-sessions stores. Those use either periodic cleanup tasks, database enforced TTLs, or expiration checks in the code.
Here's the report I sent to Google:
Summary: googleapis/nodejs-firestore-session does not enforce session expiration server-side
Product: Google OSS (open source software)
URL: https://github.com/googleapis/nodejs-firestore-session
Vulnerability type: Auth Bypass
When a user logs in they get a session cookie. The session information is written to firestore. When the cookie expires the web browser deletes the cookie (client-side only). When a cookie's expiration is manually altered client-side, this project considers it to still be valid. There is no server-side expiration checking or purging of expired sessions. As such, anyone who has ever had a session cookie can use it for long-term persistent access to the vulnerable website. Key risks include cookie theft, inability to end sessions, and inability to revoke account access.
git clone git@github.com:googleapis/nodejs-firestore-session.git
Edit the code to use one minute sessions (to make the demo easier):
index 67ed494..ce0eef7 100644
--- a/samples/quickstart.js
+++ b/samples/quickstart.js
@@ -27,6 +27,7 @@ app.use(
kind: 'express-sessions',
}),
secret: 'my-secret',
+ cookie: { maxAge: 60*1000 }, // one minute, vs default non-persistent
resave: false,
saveUninitialized: true,
})
Install, using old deps because the build is currently broken:
npm i --legacy-peer-deps
Add firebase credentials, setup a database, etc.
Run it:
node samples/quickstart.js
Open the sample page in your browser. Refresh the page a couple times to see the view counter increment. Wait more than 60 seconds. Refresh the page again. The browser has deleted the expired cookie and a fresh session is created, setting views to zero again. So far this looks like correct session management.
Edit the cookie expiration (add a couple hours, for example) using the browser tools (client-side). Wait more than 60 seconds so the cookie would normally have expired already. Refresh the page to demonstrate that the session is still active on the back-end (view count did not reset to zero). Copy the cookie to a different browser to see that cookie theft is possible, refreshing the page there will show the first browser's view count.
Session management is broken as the sessions never expire.
Review the documents saved in firestore. The expiration time is present in the documents and the expired sessions have not been deleted. Review the source code of the project. The session validation code does not consider expiration time. How to fix it
Several other session stores for express-sessions either check the expiration when validating a session, purge expired sessions via a regular cleanup task, or both. Please consider either of those approaches. Impact to other Google projects
I'm auditing projects that use this vulnerable configuration. These Google projects appear to use an insecure configuration, although I have not confirmed:
I strongly recommend checking your private source code repositories for additional examples of this issue.
Separately, the sample code recommends storing the session key in the source code. https://github.com/googleapis/nodejs-firestore-session/blob/main/samples/quickstart.js#L29 When assessing impact of this vulnerability I found a number of projects that use this weak/leaked/hard coded default key. Others have changed the value, but published their code online, leaking it again. Is it possible to avoid sample code that leaks secrets? Inexperienced developers who will simply copy/paste this code and deploy to production.
To exploit this vulnerability, the attacker needs to get access to a session cookie. This could happen through another attack. However, the attacker could also be someone who was previously authorized to access an application but no longer should have access. Typical examples include an employee who has been fired.
The attacker would be able to use whatever data or access the session cookie normally has access to. This is application specific.
For the webauthndemo project a session started with webauthn will persist long term.
Security issue will be tracked as CVE-2023-4416.
Hi, how do you deal with expired session data that has not been cleanup by express itself? For example if a user decided to deleted his browser cookies, express losses track of them, leading to garbage session data in firestore.
I'm thinking about writing a simple poller in my application code to manage this. But there is no time related information in the session document I can query for. Which would require me to query all sessions from the firestore collection. Would you mind adding a
createdAt
timestamp? This would allow me to optimise my poller query.