Page MenuHomePhabricator

Avoid implementation-defined and undefined behavior when dealing with sizes
ClosedPublic

Authored by deadalnix on Jan 17 2020, 14:26.

Details

Summary
  • Fix possible integer overflow in DER parsing

If we’re in the last loop iteration, then lenleft == 1 and it could
be the case that ret == MAX_SIZE, and so ret + lenleft will
overflow to 0 and the sanity check will not catch it. Then we will
return (int) MAX_SIZE, which should be avoided because this value is
implementation-defined. (However, this is harmless because
(int) MAX_SIZE == -1 on all supported platforms.)

  • Parse DER-enconded length into a size_t instead of an int

This avoids a possibly implementation-defined signed (int) to unsigned
(size_t) conversion portably.

  • Simplify control flow in DER parsing
  • Avoid out-of-bound pointers and integer overflows in size comparisons

This changes pointer calculations in size comparions to a form that
ensures that no out-of-bound pointers are computed, because even their
computation yields undefined behavior.
Also, this changes size comparions to a form that ensures that neither
the left-hand side nor the right-hand side can overflow.

This is a backport of secp256k1 PR578

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable