sendResetPwd, information leak

[sylvainlap]

With the sendResetPwd action, if the email address does not exist, a 404 error is thrown.

I think this is an information leak, because thanks to that call, an attacker can find if an email address is valid or not (ie in the database). I think this call should not return the user info, and send the same empty response, whatever the case (user exists or not).

Regards.

[jonnyparris]

In case it helps anyone else, I wrote a middleware handler to intercept any 400 errors as a workaround.
This way you can still benefit from the explicit error messages server side but any client side attackers/users receive a generic response:

module.exports = function(app) {
  return function(error, req, res, next) {
    let refererPath = req.headers.referer.split('/')
    refererPath = refererPath[refererPath.length-1]
    const resetPwdRequested = refererPath === 'forgot-password'
    const userNotFoundError = error && error.code === 400 && error.name === 'BadRequest' && error.message === 'User not found.'

    if (resetPwdRequested && userNotFoundError) {
      // Don't leak whether or not users exist in your DB to nefarious hackers
      // (Make sure this response is the same as when a user is found.)
      res.status(201).send({ success: true })
    }
    next(error)
  }
}

[jraut]

The same applies to error responses which result from unique violation for email. The instance details are revealed which contain the hashed password (well, any fields which were part of the original payload). This enables gathering a map of pwd => encrypt(pwd) and any other processing of the fields.

[musicformellons]

I try to find the 404 response referred above, and would expect it to show up in my chrome network tab in the status column. However I do not get any 404 (just 200) when using a not existing email. Does this mean my app is 'not vulnerable' to this kind of leaking? And why: could it be because I use websockets?

Server side I get:

error:  BadRequest: User not found.
    at new BadRequest(~/api_server/node_modules/@feathersjs/errors/lib/index.js:86:17)
    at getUserData (~/api_server/node_modules/feathers-authentication-management/lib/helpers.js:93:11)
    at ~/api_server/node_modules/feathers-authentication-management/lib/sendResetPwd.js:32:14

[claustres]

Well I am afraid sending 200 when there is actually an error is not really great, how do you let your users (and not attackers) know about any error ? To check the email an attacker can also try to register as it is often required to be unique and check this error, if no error is returned how do you tell your user that he has to provide another email ?

Regarding #85 (comment) you probably miss a removeVerification hook to avoid leaking information.

[jonnyparris]

how do you let your users (and not attackers) know about any error ?

You set a generic message that your users will receive a password reset email if their details are found in your records.

To check the email an attacker can also try to register as it is often required to be unique and check this error...

True, but generally my main aim is to slow down / prevent rapid enumeration of multiple emails - something like a captcha on your registration form can help with this.

[claustres]

Yes or rate limiting.

[eddyystop]

This issue has been moved to feathers-plus/authentication-local-management#5 and feathers-plus/authentication-local-management#6

[OnnoGabriel]

This is an old and closed issue. But in case anybody searches for a solution for this issue that works with REST and Websocket transport: You can always remove the error and unify the result that is returned to the frontend in the error and after hooks of a Feathers service. Here is a simple example for the sendResetPwd action of our auth-management' service:

  app.service('/auth-management').hooks({
    error: {
      all: [
        // Delete error and return only true after "sendResetPwd" request
        // => to hide any existence or not-existence of users
        (context) => {
          if (context.data && context.data.action === 'sendResetPwd') {
            delete context.error;
            context.result = true;
          }
        },
      ],
    },
    after: {
      all: [
        // Return only true after "sendResetPwd" request
        // => to hide any existence or not-existence of users
        (context) => {
          if (context.data && context.data.action === 'sendResetPwd') {
            context.result = true;
          }
        },
      ],
    },
  });