Skip to content

🚨opts is mutable, security issue #39

@bodinsamuel

Description

@bodinsamuel

Hello,

I recently found out that when using lookup as a function, the param opts is mutable.
That means, if you modify it, subsequent request made by same user or different users inherit the change you have made in it.
This is a very dangerous thing and as it is not documented, I suppose it was not made to be that way.

Here is simple way to reproduce:

  • use a lookup in combinaison with total
  • in the lookup method, do not return a default value
  • next user will have the total computed for the previous request and not default one set in the root object.
context('mutation', () => {
  let express;
  let app;
  let limiter;

  before(() => {
    express = require('express');
    app = express();
    limiter = subject(app, redis);
    limiter({
      path: '*',
      method: 'all',
      lookup: function(req, res, opts, next) {
        opts.lookup = 'query.api_key';
        if (req.method === 'GET') {
          opts.total = 20;
        }
        return next();
      },
      total: 3,
      expire: 1000 * 60 * 60,
    });

    app
      .get('/route', function(req, res) {
        res.send(200, 'hello');
      })
      .post('/route', function(req, res) {
        res.send(200, 'hello');
      });
  });

  it('should have a special total', function(done) {
    request(app)
      .get('/route?api_key=foobar')
      .expect('X-RateLimit-Limit', 20)
      .expect('X-RateLimit-Remaining', 19)
      .expect(200, function(e) {
        done(e);
      });
  });

  // -> This test will fail
  // X-RateLimit-Limit will be equal to 20
  it('should rollback to default', function(done) {
    request(app)
      .post('/route?api_key=foobar')
      .expect('X-RateLimit-Limit', 3)
      .expect('X-RateLimit-Remaining', 2)
      .expect(200, function(e) {
        done(e);
      });
  });
});

Has the package seems not maintained, I assume it's best to emphasis this issue 🚨
Best regards

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions