Repository navigation
Native crypto improvements: reject malformed cryptograms and failures with an exception instead of crashing the app - #210
Merged
Conversation
…shing The native decryption dereferenced the JSON values without checking them, so a cryptogram without e.g. "iv", or with a value of the wrong type, crashed the whole app with a segmentation fault instead of throwing an exception. Numbers out of the int range were cast to int, which is undefined behavior. Missing values, values of the wrong type and numbers that are no positive int now let the decryption fail with the usual exception. The JSON tree is also released if the cryptogram isn't a JSON object. Signed-off-by: binsky <timo@binsky.org>
…it; the result of the PBKDF2 key derivation is now checked as well, so a failed derivation can no longer encrypt or decrypt with an uninitialized key Signed-off-by: binsky <timo@binsky.org>
Signed-off-by: binsky <timo@binsky.org>
note: the required IV length depends on the cryptogram length (13 bytes below 64 KB, 12 below 16 MB, 11 above), so decryptccm checks it where the nonce length is calculated Signed-off-by: binsky <timo@binsky.org>
the decoding no longer exits the app when it runs out of memory, but returns NULL; it no longer leaks the Datagram struct when returning NULL Signed-off-by: binsky <timo@binsky.org>
Signed-off-by: binsky <timo@binsky.org>
the native base64 encoding exited the whole app when it ran out of memory, and returned a result without data for oversized input, which its callers didn't check. It now returns NULL in both cases, and the encryption and decryption fail with the usual exception instead Signed-off-by: binsky <timo@binsky.org>
basically just by removing two useless lines :D Signed-off-by: binsky <timo@binsky.org>
…to align with the encryptStringCpp behaviour) the input validation tests now check the exact exception and message, which are the same before and after this change Signed-off-by: binsky <timo@binsky.org>
…xt can't be created Signed-off-by: binsky <timo@binsky.org>
…n exception ( instead of encrypting with an IV and salt of leftover memory) Signed-off-by: binsky <timo@binsky.org>
…e app rare possibility, but technically possible Signed-off-by: binsky <timo@binsky.org>
…va exception Signed-off-by: binsky <timo@binsky.org>
binsky08
changed the title
binsky08
marked this pull request as ready for review
October 31, 2026 20:52
Signed-off-by: binsky <timo@binsky.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #209 (
fix/native-memory-leaks), which needs to be merged first.Malformed cryptograms or failures in the native SJCL code crashed the app. They now throw the usual
RuntimeException("Error decrypting"/"Error encrypting"). The format and JNI interface are unchanged.RAND_bytes,malloc/calloc, base64 encoding and the cipher context instead of continuing or crashingstd::bad_alloc) at the JNI boundarynullinstead of a C stringSjclInputValidationTestand a leak test for rejected decryptions🤖 AI (if applicable)