This repository was archived by the owner on Jun 3, 2026. It is now read-only.
Fix set_data_len_bytes bounds check to prevent buffer overflow - #136
Merged
Conversation
The check compared data_len_bytes against raw allocation_size, but the usable data region is allocation_size minus 24 bytes of overhead (header, canary, padding). Use AllocationSizeToMaxDataSize for the correct bound.
Compute AllocationSizeToMaxDataSize once in initialize() and store it as max_data_size. Use the cached value in set_data_len_bytes() and for the default data_len_bytes in initialize(). Also transfer it during move operations.
- Check SetEnv return value and report errors to JavaScript - Fix GetBooleanProperty to use defaultValue when property is undefined/null instead of coercing undefined to false - Enable SensitiveCobhanBufferNapi secure wipe with is_valid() guard so moved-from objects skip the wipe; add deferred-read tests to verify async decrypt data survives event loop ticks and GC - Skip verify_canaries on moved-from CobhanBuffer (cbuffer is null) - Fall back to stderr in error_log when no log hook is configured - Add set -e to integration-test.sh - Add Promise<void> type parameter to setup_async and shutdown_async
…nters - Validate SetMaxStackAllocItemSize rejects negative values instead of wrapping to a huge size_t that causes alloca stack overflow - Remove duplicate system_name prefix in log hook messages; call_log_hook already prepends it so callers should not - Null out all pointers in moved-from CobhanBuffer to prevent the destructor from dereferencing memory owned by the new object
- EndShutdownAsherah error message incorrectly said "EndSetupAsherah" - RequireParameterBuffer error messages incorrectly said "Expected String"
- Change lint glob from src/**.ts to src/**/*.ts and test/**/*.ts - Add test-specific eslint overrides for require imports and catch vars - Add set -e to update-sums.sh, replace infinite retries with bounded retries and timeouts, quote URL variable
The string length was already measured to size the allocation. Derive str_len from max_data_size instead of making a second N-API round-trip.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Summary
CobhanBuffer::set_data_len_bytesthat compared against rawallocation_sizeinstead of the usable data regionallocation_sizeminus 24 bytes of overhead (8-byte header + 8-byte canary + 8-byte safety padding)set_data_len_byteswould accept values up to 24 bytes larger than the actual data region, allowing writes past the canary into adjacent memoryTest plan