fix(digest): guard cloned state from GC in squeeze - #27
Merged
Conversation
rb_sha3_digest_squeeze clones self via rb_obj_clone to preserve the original state, but the cloned VALUE (`copy`) is not referenced again after get_sha3_digest_context extracts the raw context pointer. An optimizing compiler is then free to stop preserving `copy` on the stack, so the subsequent rb_str_new allocation can become a GC point that reclaims the clone and frees context_copy->state. The following Keccak_HashFinal then dereferences freed memory and segfaults inside the function prologue. We observed the following crash calling SHA3::Digest::SHAKE_128#hex_squeeze in a tight loop: [BUG] Segmentation fault c:0043 CFUNC :hex_squeeze /gems/.../sha3_ext.so(Keccak_HashFinal+0x10) /gems/.../sha3_ext.so(rb_sha3_digest_squeeze+0xca) Keep `copy` reachable across the allocation and finalization with RB_GC_GUARD. Also add coverage for #squeeze / #hex_squeeze, which previously had no direct tests on SHA3::Digest.
There was a problem hiding this comment.
Pull request overview
This PR fixes a Ruby C-extension GC/liveness bug in SHA3::Digest#squeeze (and therefore #hex_squeeze) by explicitly guarding the cloned digest object from being optimized out, preventing a use-after-free crash during allocation. It also adds direct spec coverage for #squeeze / #hex_squeeze behavior on SHAKE digests.
Changes:
- Add
RB_GC_GUARD(copy)inrb_sha3_digest_squeezeto keep the cloned VALUE reachable across allocation/GC. - Add RSpec coverage for
#squeeze/#hex_squeezeoutput sizing, state preservation across calls, and rejection on non-SHAKE algorithms.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
ext/sha3/digest.c |
Guards the cloned digest object to prevent GC from freeing the cloned internal state during squeeze. |
spec/sha3_digest_core_spec.rb |
Adds specs covering #squeeze/#hex_squeeze correctness and expected constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
|
@sorah Thank you for the fix! I'll push a new release shortly. Cheers! |
Contributor
Author
|
thanks for the prompt gem release, friendly reminding that you might forget to create a GitHub release 🚀 |
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.
Ensure
copyreachable across the allocation and finalization withRB_GC_GUARD.rb_sha3_digest_squeezeclones self viarb_obj_cloneto preserve the original state, but the cloned VALUE (copy) is not referenced again afterget_sha3_digest_contextextracts the raw context pointer. An optimizing compiler is then free to stop preservingcopyon the stack, so the subsequentrb_str_newallocation can become a GC point that reclaims the clone and freescontext_copy->state. The followingKeccak_HashFinalthen dereferences freed memory and segfaults inside the function prologue.We observed the following crash calling
SHA3::Digest::SHAKE_128#hex_squeezein our system:Also add coverage for #squeeze / #hex_squeeze, which previously had no direct tests on SHA3::Digest.
Disclosure: Claude Code is in use for investigation and patch suggestion. I've verified the fix fixes the crash, and have reviewed the patch before submission.