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 decryption of strings larger than 4 MiB #411

Merged
merged 5 commits into from
Sep 24, 2024

Conversation

aloisklink
Copy link
Contributor

The cloakedStringRegex fails to parse ciphertexts that are somewhere above 4 MiB large on Node.JS v20.17.0. This is due to limitations in V8's regex engine.

This causes an error:

RangeError: Maximum call stack size exceeded

I've adapted the isBase64 function from validatorjs/validator.js#503 under the MIT license, as it solves the error by using a much simpler regex.

Implementation details

By the way, I've split up this PR into:

  • a fix: commit (which fixes the bug), and
  • two feat: commits:
    • one that exports the parseCloakedString function, and another that

    • deprecates the cloakedStringRegex in favor of the parseCloakedString` function.

      This isn't actually a "feature", but Semantic Versioning 2.0.0 specifies that:

      [Minor version] MUST be incremented if any public API functionality is marked as deprecated.

This should hopefully make a really nice release note in https://github.com/47ng/cloak/releases!

The parseCloakedString needs to be exported, so that prisma-field-encryption can use it instead of the cloakedStringRegex: https://github.com/47ng/prisma-field-encryption/blob/c17354c747562857bf73fabe1c4c3a0c91380580/src/encryption.ts#L179

The `cloakedStringRegex` fails to parse ciphertexts larger than about
4 MiB. This is due to [limitations in V8's regex engine][1].

I've adapted the implementation from
validatorjs/validator.js#503 under the MIT
license, as it solves the error.

[1]: https://issues.chromium.org/issues/42207207
This function can be used instead of `cloakedStringRegex` on large
strings.
This regex fails on large 4 MiB + strings.
`parseCloakedString` should be used instead when possible.
This fails in Node.JS v20.17.0 with @47ng/cloak v1.1.0.
Copy link
Member

@franky47 franky47 left a comment

Choose a reason for hiding this comment

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

praise: LGTM, thanks for the extra care of adding tests and following SemVer!

src/message.ts Outdated Show resolved Hide resolved
src/message.ts Outdated Show resolved Hide resolved
@franky47 franky47 merged commit fc619b7 into 47ng:next Sep 24, 2024
1 check passed
Copy link

🎉 This PR is included in version 1.2.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aloisklink aloisklink deleted the fix-decryption-on-larger-strings branch September 24, 2024 12:54
Copy link

🎉 This PR is included in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants