Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

convert salt to Buffer instance before encrypting #2950

Merged
merged 2 commits into from
Jul 17, 2019

Conversation

michaelsbradleyjr
Copy link
Contributor

Previously, if a salt option was supplied it would be passed to pbkdf2Sync/scrypt as a string, but during decryption it was always converted to a Buffer instance such that supplying a salt option resulted in output that could not be decrypted.

This commit fixes the bug, and also makes encrypted output match up with the output of other wallet libraries, e.g. ethers, whenever the equivalent encryption options are used consistently across libraries.

Previously, if a `salt` option was supplied it would be passed to
`pbkdf2Sync`/`scrypt` as a string, but during decryption it was always
converted to a Buffer instance such that supplying a `salt` option resulted in
output that could not be decrypted.

This commit fixes the bug, and also makes encrypted output match up with the
output of other wallet libraries, e.g.  `ethers`, whenever the equivalent
encryption options are used consistently across libraries.
@coveralls
Copy link

coveralls commented Jul 17, 2019

Coverage Status

Coverage increased (+0.1%) to 95.636% when pulling d326043 on michaelsbradleyjr:fix/accounts-decrypt into e95e99b on ethereum:2.x.

@nivida nivida added 2.x 2.0 related issues Bug Addressing a bug In Progress Currently being worked on labels Jul 17, 2019
@nivida nivida removed the In Progress Currently being worked on label Jul 17, 2019
@nivida nivida merged commit 20689fe into web3:2.x Jul 17, 2019
michaelsbradleyjr added a commit to michaelsbradleyjr/ethereumjs-wallet that referenced this pull request Jul 17, 2019
Previously, if a `salt` option was supplied as a string it would be passed to
`pbkdf2Sync`/`scrypt` as a string, but during decryption it was always
converted to a Buffer instance such that supplying a `salt` option as a string
resulted in output that could not be decrypted.

This commit fixes the bug, and also makes encrypted output match up with the
output of other wallet libraries, e.g.  `ethers`, whenever the equivalent
encryption options are used consistently across libraries.

See the following `web3.js` PRs: [#2950][2950] and [#2938][2938]

[2950]: web3/web3.js#2950
[2938]: web3/web3.js#2938
@michaelsbradleyjr
Copy link
Contributor Author

See also: ethereumjs/ethereumjs-wallet#95.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2.x 2.0 related issues Bug Addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants