Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLAID-SECRET is leaked in error objects #565

Open
meshuamam opened this issue Feb 6, 2023 · 6 comments
Open

PLAID-SECRET is leaked in error objects #565

meshuamam opened this issue Feb 6, 2023 · 6 comments

Comments

@meshuamam
Copy link

When receiving a 400 error, using the Plaid client, the error thrown will contain the PLAID-SECRET key. Since it's a common practice to log an error object, this will often result in PLAID-SECRET being leaked to logs.

Consider the following scenario:

const configuration = new Configuration({
      basePath: this.config.basePath,
      baseOptions: {
        headers: {
          'PLAID-CLIENT-ID': this.config.clientId,
          'PLAID-SECRET': this.config.secret,
        },
      },
    })

    this.client = new PlaidApi(configuration)

try {
      const response = await this.client.linkTokenCreate({
        client_name: clientName,
        country_codes: [CountryCode.Us],
        language: 'en',
        user: {
          client_user_id: clientUserId,
        },
        products: this.config.products,
      })

      this.logger.info('Plaid link token created', {
        clientName,
        env: this.config.environment,
      })

      return response.data.link_token
    } catch (err) {
      this.logger.error(err)
      throw err
    }

In this case, if we receive a 400 from the server (for example because the client_user_id is missing, or any other reason), err.config.headers will include PLAID-SECRET.
I haven't tested this with other error codes.

@phoenixy1
Copy link
Contributor

Thank you for the report! I have sent this to our security team for prioritization. We will be primarily tracking work on this issue on our internal tracker, but I have subscribed to the issue in our internal Jira and will do my best to remember to update it here once it's fixed.

@donleistman
Copy link

@phoenixy1 Any updates from the security team? I stumbled upon this issue and found that it affects us as well.

If removing the secret from the error response is not possible or will take a while, it would be good to at least put a page in the docs warning against logging the full error.

@phoenixy1
Copy link
Contributor

@donleistman Security evaluated this issue and agreed it should be fixed, but at a priority level that does not have an SLA associated, so we don't have an ETA to share on the fix. I'll update the README as suggested.

phoenixy1 added a commit that referenced this issue May 12, 2023
Per comments in #565
@donleistman
Copy link

Thanks @phoenixy1 ! Much appreciated

@donleistman
Copy link

Hey @phoenixy1 I noticed that the warning you added to the README was removed. Do you know if the issue was fixed?

@phoenixy1
Copy link
Contributor

Sorry, the issue was that I forgot that the README files are generated from a template so the changes got overwritten when the README was regenerated. I've made the update to the template file for the README so it'll get propagated the next time it gets regenerated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants