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

Replace tweetnacl by @noble/curves #1209

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

gtsonevv
Copy link
Collaborator

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • Commit messages follow the conventional commits spec
  • If this is a code change: I have written unit tests.
  • If this changes code in a published package: I have run pnpm changeset to create a changeset JSON document appropriate for this change.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #1199) and the maintainers have approved on my working plan.

Motivation

This PR aims to replace tweetnacl with @noble/curves to improve the security of near-api-js. The reason is that tweetnacl has a problem called signature malleability. This means that someone could create a different signature for the same message without knowing the secret key. This could lead to vulnerabilities in certain situations. @noble/curves doesn't have this issue, making it a more secure choice for our cryptographic needs. Additionally, @noble/curves is much smaller than tweetnacl. It’s used in a lot of Ethereum projects including Metamask and protonmail, so it should be reliable replacement.

Test Plan

The existing keypair generation, signature generation and validation tests are passing.

Related issues/PRs

#1199

@gtsonevv gtsonevv requested a review from frol November 24, 2023 12:40
Copy link

changeset-bot bot commented Nov 24, 2023

🦋 Changeset detected

Latest commit: cd351b2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@near-js/accounts Patch
@near-js/crypto Patch
near-api-js Patch
@near-js/providers Patch
@near-js/cookbook Patch
@near-js/wallet-account Patch
@near-js/biometric-ed25519 Patch
@near-js/keystores-browser Patch
@near-js/keystores-node Patch
@near-js/keystores Patch
@near-js/signers Patch
@near-js/transactions Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@gtsonevv gtsonevv merged commit d594279 into near:master Dec 15, 2023
3 checks passed
This was referenced Dec 15, 2023
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

Successfully merging this pull request may close these issues.

Security: ed25519 verification accepts forged signatures
2 participants