fireblocks / fireblocks-sdk-js

Typescript & Javascript SDK for developers using Fireblocks API
https://docs.fireblocks.com/api/swagger-ui/
MIT License
72 stars 69 forks source link

Downgrade jsonwebtoken version to 8.5.1 #297

Open alexandre-tsu-manuel opened 1 month ago

alexandre-tsu-manuel commented 1 month ago

Pull Request Description

In PR #132 , the Snyk bot suggested to update the jsonwebtoken module from 8.5.1 to 9.0.0. Merging it was a bad decision.

JWT 9.0.0 has bugs, notably, it doesn't work when it's used in a browser. See:

The reason why JWT was updated in fireblocks SDK in the first place is nonsensical. This is to fix a security issue, linked in the PR: https://security.snyk.io/vuln/SNYK-JS-JSONWEBTOKEN-3180022

This security issue mentions the possibility to bypass the signature process when giving a null private key for example. This is not a problem, as the only usage of that module in the fireblocks sdk is to sign data before calling an API: https://github.com/search?q=repo%3Afireblocks%2Ffireblocks-sdk-js%20jsonwebtoken&type=code

Should the signature be bypassed, the API call just won't work, nothing more. No security issue here.

Choosing to update JWT on the other hand, that prevents using the SDK in a browser, which has many interesting use cases. When trying to integrate, an exception is raised at the first API call:

provider.ts:225 Uncaught (in promise) Error: Fireblocks SDK Error: Right-hand side of 'instanceof' is not an object
    at FireblocksWeb3Provider.createFireblocksError (provider.ts:225:17)
    at FireblocksWeb3Provider.populateAccounts (provider.ts:205:20)

No fix to this has been proposed by the JWT project. The only way to avoid it is to downgrade or use something else.

Therefore, I suggest rolling back this upgrade.

Type of change

How Has This Been Tested?

The bug was discovered when trying to use the code provided at https://developers.fireblocks.com/reference/evm-web3-provider Downgrading the JWT package fixes the exception mentionned above.

Checklist: