Skip to content

Commit

Permalink
[SECP256K1] Return NULL early in context_preallocated_create if flags…
Browse files Browse the repository at this point in the history
… invalid

Summary:
```
If the user passes invalid flags to _context_create, and the default
illegal callback does not abort the program (which is possible), then we
work with the result of malloc(0), which may be undefined behavior. This
violates the promise that a library function won't crash after the
illegal callback has been called.

This commit fixes this issue by returning NULL early in _context_create
in that case.
```

Backport of secp256k1 [[bitcoin-core/secp256k1#840 | PR840]].

Test Plan:
  ninja check-secp256k1

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D8202
  • Loading branch information
real-or-random authored and Fabcien committed Oct 31, 2020
1 parent c6b1044 commit 9acf260
Showing 1 changed file with 9 additions and 7 deletions.
16 changes: 9 additions & 7 deletions src/secp256k1/src/secp256k1.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_no_pr

size_t secp256k1_context_preallocated_size(unsigned int flags) {
size_t ret = ROUND_TO_ALIGN(sizeof(secp256k1_context));
/* A return value of 0 is reserved as an indicator for errors when we call this function internally. */
VERIFY_CHECK(ret != 0);

if (EXPECT((flags & SECP256K1_FLAGS_TYPE_MASK) != SECP256K1_FLAGS_TYPE_CONTEXT, 0)) {
secp256k1_callback_call(&default_illegal_callback,
Expand Down Expand Up @@ -122,21 +124,21 @@ secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigne
if (!secp256k1_selftest()) {
secp256k1_callback_call(&default_error_callback, "self test failed");
}
VERIFY_CHECK(prealloc != NULL);

prealloc_size = secp256k1_context_preallocated_size(flags);
if (prealloc_size == 0) {
return NULL;
}
VERIFY_CHECK(prealloc != NULL);
ret = (secp256k1_context*)manual_alloc(&prealloc, sizeof(secp256k1_context), base, prealloc_size);
ret->illegal_callback = default_illegal_callback;
ret->error_callback = default_error_callback;

if (EXPECT((flags & SECP256K1_FLAGS_TYPE_MASK) != SECP256K1_FLAGS_TYPE_CONTEXT, 0)) {
secp256k1_callback_call(&ret->illegal_callback,
"Invalid flags");
return NULL;
}

secp256k1_ecmult_context_init(&ret->ecmult_ctx);
secp256k1_ecmult_gen_context_init(&ret->ecmult_gen_ctx);

/* Flags have been checked by secp256k1_context_preallocated_size. */
VERIFY_CHECK((flags & SECP256K1_FLAGS_TYPE_MASK) == SECP256K1_FLAGS_TYPE_CONTEXT);
if (flags & SECP256K1_FLAGS_BIT_CONTEXT_SIGN) {
secp256k1_ecmult_gen_context_build(&ret->ecmult_gen_ctx, &prealloc);
}
Expand Down

0 comments on commit 9acf260

Please sign in to comment.