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

Fix Incorrect Usage of bn.js API in browser.js and Ensure Backward Compatibility for Decryption #96

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mystiko0x1
Copy link

@mystiko0x1 mystiko0x1 commented Feb 5, 2024

This PR addresses an incorrect usage of the bn.js library within browser.js on the master branch, specifically in how BN objects are converted to buffers. The existing code uses Px.toArray() method, which can lead to variable length byte arrays, potentially causing inconsistent behavior:

// Current incorrect implementation
resolve(Buffer.from(Px.toArray()));

To ensure consistent byte array lengths, we should leverage Buffer.from(Px.toArray(undefined, 32)) method, specifying a fixed output length. The proposed change is as follows:

// Corrected implementation
resolve(Buffer.from(Px.toArray(undefined, 32)));

This modification guarantees that the conversion yields a buffer of a fixed size, mitigating issues encountered with variable length arrays.

Additionally, this PR introduces a backward compatibility layer for decrypting messages encrypted with previously derived incorrect keys. The solution attempts decryption using the corrected key length; should this fail, it then falls back to the previously incorrect key length by trimming leading zeros. This dual-path approach ensures that both newly encrypted messages and those encrypted under the prior flawed logic remain accessible.

Related Issues: This fix is in response to issues encountered by users, documented in #90, #81, #63, and #52 providing a comprehensive solution to the encryption key length inconsistencies observed.

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.

1 participant