Merge bitcoin-core/secp256k1#1910: scratch: reject sizes that overflow when added to header
What the developer wrote
Authored by merge-script
Merge bitcoin-core/secp256k1#1910: scratch: reject sizes that overflow when added to header
3d4340d17325a6732108423f9d3e54de99539a60 scratch: reject sizes that overflow when added to header (Sebastian Falbesoner)
Pull request description:
We only use scratch space internally, so this is not an issue for the user (scratch API functions have been removed since 0.6.0, see #1620), but fixing this might still make sense to prepare for the unlikely case that we expose the scratch API again in the future. An alternative could be to simply delete the scratch space functionality already, as e.g. done in PR #1789.
Reported by [Project Loupe](https://github.com/project-loupe)
ACKs for top commit:
apoelstra:
utACK 3d4340d17325a6732108423f9d3e54de99539a60
real-or-random:
utACK 3d4340d17325a6732108423f9d3e54de99539a60
Tree-SHA512: 8e380919a48445d8df46a568e4fcbcbe31b5b5668586942d1eaa1b90b4e0669548d5ca831c8dde484b869c7e95faacb608491738a59c5e1ee15d4084acc7ead6
What changed, and why it matters
This commit fixes an integer overflow bug in the library's internal scratch-space memory allocator. If a caller requested a scratch space with a size near the maximum possible value, adding the allocator's own bookkeeping header could wrap around to a tiny total size. That tiny allocation would then be used as if it were huge, potentially causing memory corruption. However, the scratch-space API is no longer exposed to users, so normal applications cannot trigger this.
No urgent action for end users: the scratch API is internal and not reachable from public interfaces. Developers integrating libsecp256k1 should ensure they are on a version containing this fix if they build with internal symbols exposed or if the scratch API is reintroduced in the future.
Security signals we found
Integer overflow in size calculation
Heap allocation size mismatch
Potential buffer overflow / out-of-bounds write
Internal-only API reduces practical exposure
Evidence from the diff
In secp256k1_scratch_create, the code previously computed base_alloc + size without checking for overflow, where base_alloc is the aligned size of the secp256k1_scratch header. On platforms where size_t wraps, passing a very large size (e.g., SIZE_MAX or SIZE_MAX - base_alloc + 1) causes checked_malloc to allocate a small buffer, while subsequent code treats the requested size as available. The patch rejects sizes where size > SIZE_MAX - base_alloc and returns NULL. Tests are added for SIZE_MAX and the smallest wrapping size.
Changed components
src/scratch_impl.hsecp256k1_scratch_createinternal scratch-space allocatorInspect captured patch +14 / −2
### src/scratch_impl.h
@@ -12,8 +12,14 @@
static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* error_callback, size_t size) {
const size_t base_alloc = ROUND_TO_ALIGN(sizeof(secp256k1_scratch));
- void *alloc = checked_malloc(error_callback, base_alloc + size);
- secp256k1_scratch* ret = (secp256k1_scratch *)alloc;
+ void *alloc;
+ secp256k1_scratch* ret;
+ /* Reject sizes that would wrap when added to the aligned header. */
+ if (size > SIZE_MAX - base_alloc) {
+ return NULL;
+ }
+ alloc = checked_malloc(error_callback, base_alloc + size);
+ ret = (secp256k1_scratch *)alloc;
if (ret != NULL) {
memset(ret, 0, sizeof(*ret));
memcpy(ret->magic, "scratch", 8);
### src/tests.c
@@ -420,6 +420,12 @@ static void run_scratch_tests(void) {
CHECK(secp256k1_scratch_alloc(&CTX->error_callback, scratch, SIZE_MAX) == NULL);
secp256k1_scratch_space_destroy(CTX, scratch);
+ /* Creating a scratch space whose size would wrap around when the aligned
+ * header size is added to it fails, both for SIZE_MAX and for the smallest
+ * size that still wraps. */
+ CHECK(secp256k1_scratch_space_create(CTX, SIZE_MAX) == NULL);
+ CHECK(secp256k1_scratch_space_create(CTX, SIZE_MAX - ROUND_TO_ALIGN(sizeof(secp256k1_scratch)) + 1) == NULL);
+
/* cleanup */
secp256k1_scratch_space_destroy(CTX, NULL); /* no-op */
}Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.