fastify/sessions does not destroy automatically the sessions which already expired.

[Prag1974]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.27

Plugin version

10.8.0

Node.js version

20.12.0

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

11

Description

Fastify version: latest
Node.js version: 20.12.0
NPM version: 10.2.0

For other versions please check out the package.json file

I'm using TypeScript and I'm trying to build a session-based-authentication system. I used to use express, but I've found many reasons why fastify is much better. So I'm using fastify/session as a session package. I'm using Prisma to store sessions. I've connected prism as the session store of fastify/session. It works fine.

But the problem is that fastify/session does not delete expired sessions. Neither from memory nor from Prisma. Only cookies are deleted and that is thanks to fastify/cookies. Instead of deleting the session from the fastify/session store, it gives the user a new session. You can say that there is no problem here, after all, it does not use the old session. But when we replace the cookie with the cookie of the old session and advance the expire time of the cookie (because fastify/cookies removes the cookie if we don't advance it), it still allows us to access the old session. (Check the video) This is a security vulnerability. How can I solve this?

And its not about the session storage which I implement (prisma storage), if you delete this storage (that means you are using memory storage by default) same vulnerability is still valid. How can I solve this? or any source can you suggest?

Check index.ts file, the related video and package.json file.

If fastify/session automatically destroys expired sessions, there will be no problem, but the destroy method I implemented in the store is never triggered. Can you help me?

Apologies for my english. Thanks.

Video (or you can also watch on youtube: https://youtu.be/EXB0mhckHoc):

Accessingoldsession.mp4

index.ts (which also available to inspect in github : GitHub Repo):

import Fastify, { FastifyReply, FastifyRequest, Session } from "fastify";
import fastifyCookie from "@fastify/cookie";
import fastifySession from "@fastify/session";
import { db } from "./libs/db";

declare module "fastify" {
  interface Session {
    gotItFromBaseURL?: boolean; //This is just for test.
    gotItFromTestURL?: boolean;
  }
}

const fastify = Fastify();

const PORT = 3000;

fastify.register(fastifyCookie, {});
fastify.register(fastifySession, {
  store: { //If you dont implement your own storage and use default one, same vulnerability is still valid.
    async set(
      sessionId: string,
      session: Session,
      callback: (err?: any) => void
    ) {
      try {
        await db.session.upsert({
          where: {
            id: sessionId,
          },
          update: {
            data: session as any,
          },
          create: {
            id: sessionId,
            data: session as any,
          },
        });
        return callback(undefined);
      } catch (error) {
        return callback(error);
      }
    },
    async get(
      sessionId: string,
      callbackSession: (err: any, result?: Session) => void
    ) {
      try {
        const storedSession = await db.session.findUnique({
          where: { id: sessionId },
        });
        if (!storedSession) throw Error("Session couldn't found!");
        
        const session: Session = {
          ...(storedSession.data as any),
        };

        callbackSession(undefined, session);
      } catch (error) {
        callbackSession(error, undefined);
      }
    },
    async destroy(sessionId: string, callback: (err?: any) => void) {
      try {
        console.log("Destroy fired");
        const deleted = await db.session.delete({ where: { id: sessionId } });
        if (!deleted) throw Error("Couldnt found to destroy a session!");

        callback(undefined);
      } catch (error) {
        callback(error);
      }
    },
  },
  secret: "supersecretkeysupersecretkeysupersecretkeysupersecretkey",
  cookie: {
    secure: false,
    maxAge: 1000 * 15, // 15 seconds
  },
});

fastify.get("/", (req: FastifyRequest, res: FastifyReply) => {
  req.session.gotItFromBaseURL = true;
  console.log(req.session.sessionId);
  return res.send(req.session);
});

try {
  fastify.listen({ port: PORT });
  console.log(`Server is listening port ${PORT}`);
} catch (error) {
  fastify.log.error(error);
  process.exit(1);
}

Link to code that reproduces the bug

No response

Expected Behavior

No response

[rktyt]

Automatic deletion is not provided for @fastify/session, just as it is not provided for express-session.

The following implementations should therefore be carried out.

• Database records should include an expires at
• Check expires in store's get method
• Delete expired data periodically

---

Reference package:
https://www.npmjs.com/package/@quixo3/prisma-session-store

Notes:
The above package is not likely to be available as is due to peerDependencies. ( Same as #224 )

---

Depending on the database you use, it can be automatically deleted on the database side, so I think the session library does not take care of database-specific matters.
eg) For AWS dynamodb, just set TTL

[Prag1974]

Automatic deletion is not provided for @fastify/session, just as it is not provided for express-session.

The following implementations should therefore be carried out.

• Database records should include an expires at
• Check expires in store's get method
• Delete expired data periodically

---

Reference package:
https://www.npmjs.com/package/@quixo3/prisma-session-store

Notes:
The above package is not likely to be available as is due to peerDependencies. ( Same as #224 )

---

Depending on the database you use, it can be automatically deleted on the database side, so I think the session library does not take care of database-specific matters.
eg) For AWS dynamodb, just set TTL

Express-sessions automatically deletes the expired sessions. And if the fastify/sessions does not destroy them automatically, then what is the purpose of implementing destroy which the mandatory method for implementing your own session store (set, get, destroy), if it doesn't make sense, why fastify/sessions get us to implement it, what is the purpose? And checking if the session is expired in EVERY get request is unnecessary probably for 9 of 10 requests. And this is bad for the system. I guess it would be devouring for the server.

[rktyt]

The memory store of express-session@1.18.0 is not deleted unless the all method is called.
Data that is not sent from the browser will not be deleted.

---

destroy is called in onRequest hook.
destroy is used when a cookie has an expiration date-time set and expires on the server side after the browser sends the cookie.
Not called if you do not set the maxAge or expires options.
https://github.com/fastify/session/blob/v10.8.0/lib/fastifySession.js
Also, it seems that destroy is required to make store compatible with express-session.

---

I would like to hear the views of this library's contributors.
I feel like the memory store implementation is too simple.
However, it is clearly stated that it should not be used in production, so I feel like this is fine.
If you think it would be better to include precautions and implementation policies when using a database in the document, please update it.

[mcollina]

I would need to investigate this.

The cat is out of the bag, but please submit all potential vulnerabilities through proper channels and not via GH issues.

[rktyt]

@Prag1974 @mcollina
Sorry, the code reading was not very good.

Apparently there is a bug in the handling of cookie.maxAge.

https://github.com/fastify/session/blob/v10.8.0/lib/fastifySession.js#L85-L105
expires is checked in the above code.
However, expires is updated when restoring.
https://github.com/fastify/session/blob/v10.8.0/lib/session.js#L43
https://github.com/fastify/session/blob/v10.8.0/lib/cookie.js#L15
https://github.com/fastify/session/blob/v10.8.0/lib/cookie.js#L42

Therefore, if the cookie.maxAge option is set, the expiration check will not work.
The check works if you set the cookie.expires option.

[Prag1974]

I would need to investigate this.

The cat is out of the bag, but please submit all potential vulnerabilities through proper channels and not via GH issues.

Where are the proper channels, ? I will record a new video in details and appeal there with it.

[Prag1974]

@Prag1974 @mcollina
Sorry, the code reading was not very good.

Apparently there is a bug in the handling of cookie.maxAge.

https://github.com/fastify/session/blob/v10.8.0/lib/fastifySession.js#L85-L105
expires is checked in the above code.
However, expires is updated when restoring.
https://github.com/fastify/session/blob/v10.8.0/lib/session.js#L43
https://github.com/fastify/session/blob/v10.8.0/lib/cookie.js#L15
https://github.com/fastify/session/blob/v10.8.0/lib/cookie.js#L42

Therefore, if the cookie.maxAge option is set, the expiration check will not work.
The check works if you set the cookie.expires option.

I know. But that is not the case. When the fastify session gives us a new session. That means the old one should be expired. But as in the video. We can access it and this is a vulnerability. Did you watch the video?

[mcollina]

I would need to investigate this.
The cat is out of the bag, but please submit all potential vulnerabilities through proper channels and not via GH issues.

Where are the proper channels, ? I will record a new video in details and appeal there with it.

https://github.com/fastify/session/security/policy. Even asking “where can I send a potential security issue privately” is better.

[mcollina]

Fixed as part of GHSA-pj27-2xvp-4qxg.