Differential D8265

[SECP256K1] Prevent arithmetic on NULL pointer if the scratch space is too small
ClosedPublic
Actions

Authored by Fabien on Nov 4 2020, 16:10.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABCfb908c83a6a5: [SECP256K1] Prevent arithmetic on NULL pointer if the scratch space is too small

Summary

If the scratch space is too small when calling
`secp256k1_ecmult_strauss_batch()`, the `state.pre_a` allocation will
fail and the pointer will be `NULL`. This causes `state.pre_a_lam` to be
computed from the `NULL` pointer.

It is also possible that the first allocation to fail is for `state.ps`,
which will cause the failure to occur when in
`secp256k1_ecmult_strauss_wnaf()`.

The issue has been detected by UBSAN using Clang 10:

CC=clang \
CFLAGS="-fsanitize=undefined -fno-omit-frame-pointer" \
LDFLAGS="-fsanitize=undefined -fno-omit-frame-pointer" \
../configure

UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1 make check

Backport of secp256k1 PR839.

Test Plan

With Clang and UBSAN:

ninja check-secp256k1

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Nov 4 2020, 16:10

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 4 2020, 16:11

Fabien requested review of this revision.Nov 4 2020, 16:11

teamcity edited the summary of this revision. (Show Details)Nov 4 2020, 16:11

Fabien retitled this revision from Prevent arithmetic on NULL pointer if the scratch space is too small to [SECP256K1] Prevent arithmetic on NULL pointer if the scratch space is too small.Nov 4 2020, 16:11

Harbormaster completed remote builds in B13852: Diff 25420.Nov 4 2020, 16:22

deadalnix accepted this revision.Nov 4 2020, 16:30

This revision is now accepted and ready to land.Nov 4 2020, 16:30

Closed by commit rABCfb908c83a6a5: [SECP256K1] Prevent arithmetic on NULL pointer if the scratch space is too small (authored by Fabien). · Explain WhyNov 4 2020, 16:52

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABCfb908c83a6a5: [SECP256K1] Prevent arithmetic on NULL pointer if the scratch space is too small.