Skip to content

Detect invalid private keys when importing them#1007

Merged
csillag merged 2 commits intomasterfrom
csillag/detect-invalid-private-key
Sep 21, 2022
Merged

Detect invalid private keys when importing them#1007
csillag merged 2 commits intomasterfrom
csillag/detect-invalid-private-key

Conversation

@csillag
Copy link
Copy Markdown
Contributor

@csillag csillag commented Sep 21, 2022

Fixes #960.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 21, 2022

MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ EDITORCONFIG editorconfig-checker 2 0 0.01s
✅ GIT git_diff yes no 0.01s
✅ TYPESCRIPT eslint 2 0 0 6.08s

See errors details in artifact MegaLinter reports on CI Job page
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 21, 2022

Codecov Report

Merging #1007 (7990cc4) into master (6e2f286) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 7990cc4 differs from pull request most recent head 3d94011. Consider uploading reports for the commit 3d94011 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1007      +/-   ##
==========================================
+ Coverage   88.71%   88.76%   +0.05%     
==========================================
  Files         101      101              
  Lines        1754     1763       +9     
  Branches      405      406       +1     
==========================================
+ Hits         1556     1565       +9     
  Misses        198      198              
Flag Coverage Δ
cypress 60.46% <100.00%> (+0.20%) ⬆️
jest 79.71% <90.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/app/lib/key.ts 93.75% <100.00%> (+8.03%) ⬆️

@csillag
Copy link
Copy Markdown
Contributor Author

csillag commented Sep 21, 2022

Here is how to test this:

1. Reproduce the error on master branch

  • Go to http://localhost:3000/open-wallet/private-key
  • Enter bad private key: aaamZybIOymrQCpCGGICczsaopANP02kwOhCyxETXljLLmRChL1QJGzJq3Pf3i+dFBN+peIK2vQ3Ew0wSQbp3g== (this should seemingly work)
  • Try to initiate a transaction (you should get an error)

2. Try the same thing on this branch

  • Importing the bad key should give you an error message
  • Importing a good key still works as before: oPNmZybIOymrQCpCGGICczsaopANP02kwOhCyxETXljLLmRChL1QJGzJq3Pf3i+dFBN+peIK2vQ3Ew0wSQbp3g==

@csillag csillag requested a review from lukaw3d September 21, 2022 16:13
Copy link
Copy Markdown
Contributor

@pro-wh pro-wh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reasonable way to check

Comment thread src/app/lib/key.ts Outdated
Comment thread src/app/lib/key.ts Outdated
@csillag csillag force-pushed the csillag/detect-invalid-private-key branch from 7990cc4 to 3d94011 Compare September 21, 2022 23:19
@csillag csillag merged commit bec4846 into master Sep 21, 2022
@csillag csillag deleted the csillag/detect-invalid-private-key branch September 21, 2022 23:44
Copy link
Copy Markdown
Contributor

@lukaw3d lukaw3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Comment thread src/app/lib/key.ts
const testMessage = Uint8Array.from([1, 2, 3])
const signedTestMessage = nacl.sign(testMessage, secretKey)
const testDecoded = nacl.sign.open(signedTestMessage, publicKey)
return `${testDecoded}` === `${testMessage}`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the implicit string conversion - explicit .join(',') would be much better. But I would use uint2hex.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this has been improved in #1009

csillag added a commit that referenced this pull request Sep 22, 2022
csillag added a commit that referenced this pull request Sep 22, 2022
... when doing UInt8Array to string conversion.

This improves upon #1007.
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.

Can we detect typos in private key?

3 participants